[PATCH v5 00/12] Simple IPC Mechanism
- From
- Jeff Hostetler via GitGitGadget <gitgitgadget@gmail.com>
- Date
- Mar 9, 2021, 15:02 UTC
- Message-ID
- <pull.766.v5.git.1615302157.gitgitgadget@gmail.com>
- In-Reply-To
- <pull.766.v4.git.1613598529.gitgitgadget@gmail.com>
Here is V5 of my "Simple IPC" series. This version is a reiteration of the series and combines the original "Simple IPC Mechanism V4" series [1] and the "Simple IPC Cleanups V1" series [2]. It squashes them together and then includes responses to last minute comments received on both.
These include: (a) Rename PACKET_READ_NEVER_DIE to PACKET_READ_GENTLE_ON_READ_ERROR. (b) Accept zero length timeout in unix_stream_socket__conect(). Only supply the default timeout when a negative value is passed. Make the default timeout value private to the .c file.
I apologize for rerolling something that is in "next". I think the combined result is better long term than preserving them as two sequential series. Given where we are in the release cycle, I thought it best to have a cleaned up series for consideration post v2.31.
[1] Upstream in jh/simple-ipc via https://github.com/gitgitgadget/git/pull/766 (and in "next" relative to v2.30.1) https://lore.kernel.org/git/pull.766.v4.git.1613598529.gitgitgadget@gmail.com/T/#mbd1da5ff93ef273049090f697aeab68c74f698f1
[2] Upstream in `jh/simple-ipc-cleanups via https://github.com/gitgitgadget/git/pull/893 https://lore.kernel.org/git/8ea6401c-6ee6-94cb-4e33-9dfffaf466e8@jeffhostetler.com/T/#t
Jeff Hostetler (9):
pkt-line: eliminate the need for static buffer in
packet_write_gently()
simple-ipc: design documentation for new IPC mechanism
simple-ipc: add win32 implementation
unix-socket: eliminate static unix_stream_socket() helper function
unix-socket: add backlog size option to unix_stream_listen()
unix-socket: disallow chdir() when creating unix domain sockets
unix-stream-server: create unix domain socket under lock
simple-ipc: add Unix domain socket implementation
t0052: add simple-ipc tests and t/helper/test-simple-ipc toolJohannes Schindelin (3): pkt-line: do not issue flush packets in write_packetized_*() pkt-line: add PACKET_READ_GENTLE_ON_READ_ERROR option pkt-line: add options argument to read_packetized_to_strbuf()
Documentation/technical/api-simple-ipc.txt | 105 +++ Makefile | 9 + builtin/credential-cache--daemon.c | 3 +- builtin/credential-cache.c | 2 +- compat/simple-ipc/ipc-shared.c | 28 + compat/simple-ipc/ipc-unix-socket.c | 986 +++++++++++++++++++++ compat/simple-ipc/ipc-win32.c | 751 ++++++++++++++++ config.mak.uname | 2 + contrib/buildsystems/CMakeLists.txt | 8 +- convert.c | 11 +- pkt-line.c | 58 +- pkt-line.h | 17 +- simple-ipc.h | 239 +++++ t/helper/test-simple-ipc.c | 787 ++++++++++++++++ t/helper/test-tool.c | 1 + t/helper/test-tool.h | 1 + t/t0052-simple-ipc.sh | 122 +++ unix-socket.c | 53 +- unix-socket.h | 12 +- unix-stream-server.c | 128 +++ unix-stream-server.h | 36 + 21 files changed, 3307 insertions(+), 52 deletions(-) create mode 100644 Documentation/technical/api-simple-ipc.txt create mode 100644 compat/simple-ipc/ipc-shared.c create mode 100644 compat/simple-ipc/ipc-unix-socket.c create mode 100644 compat/simple-ipc/ipc-win32.c create mode 100644 simple-ipc.h create mode 100644 t/helper/test-simple-ipc.c create mode 100755 t/t0052-simple-ipc.sh create mode 100644 unix-stream-server.c create mode 100644 unix-stream-server.h
base-commit: f01623b2c9d14207e497b21ebc6b3ec4afaf4b46 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-766%2Fjeffhostetler%2Fsimple-ipc-v5 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-766/jeffhostetler/simple-ipc-v5 Pull-Request: https://github.com/gitgitgadget/git/pull/766
Range-diff vs v4:
1: 2d6858b1625a ! 1: 311ea4a5cd71 pkt-line: eliminate the need for static buffer in packet_write_gently()
@@ Commit message
static buffer, thread-safe scratch space, or an excessively large stack
buffer.
- Change the API of `write_packetized_from_fd()` to accept a scratch space
- argument from its caller to avoid similar issues here.
+ Change `write_packetized_from_fd()` to allocate a temporary buffer rather
+ than using a static buffer to avoid similar issues here.
These changes are intended to make it easier to use pkt-line routines in
a multi-threaded context with multiple concurrent writers writing to
@@ Commit message
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
- ## convert.c ##
-@@ convert.c: static int apply_multi_file_filter(const char *path, const char *src, size_t len
- if (err)
- goto done;
-
-- if (fd >= 0)
-- err = write_packetized_from_fd(fd, process->in);
-- else
-+ if (fd >= 0) {
-+ struct packet_scratch_space scratch;
-+ err = write_packetized_from_fd(fd, process->in, &scratch);
-+ } else
- err = write_packetized_from_buf(src, len, process->in);
- if (err)
- goto done;
-
## pkt-line.c ##
@@ pkt-line.c: int packet_write_fmt_gently(int fd, const char *fmt, ...)
@@ pkt-line.c: int packet_write_fmt_gently(int fd, const char *fmt, ...)
return 0;
}
@@ pkt-line.c: void packet_buf_write_len(struct strbuf *buf, const char *data, size_t len)
- packet_trace(data, len, 1);
- }
--int write_packetized_from_fd(int fd_in, int fd_out)
-+int write_packetized_from_fd(int fd_in, int fd_out,
-+ struct packet_scratch_space *scratch)
+ int write_packetized_from_fd(int fd_in, int fd_out)
{
- static char buf[LARGE_PACKET_DATA_MAX];
++ char *buf = xmalloc(LARGE_PACKET_DATA_MAX);
int err = 0;
ssize_t bytes_to_write;
while (!err) {
- bytes_to_write = xread(fd_in, buf, sizeof(buf));
-+ bytes_to_write = xread(fd_in, scratch->buffer,
-+ sizeof(scratch->buffer));
- if (bytes_to_write < 0)
+- if (bytes_to_write < 0)
++ bytes_to_write = xread(fd_in, buf, LARGE_PACKET_DATA_MAX);
++ if (bytes_to_write < 0) {
++ free(buf);
return COPY_READ_ERROR;
++ }
if (bytes_to_write == 0)
break;
-- err = packet_write_gently(fd_out, buf, bytes_to_write);
-+ err = packet_write_gently(fd_out, scratch->buffer,
-+ bytes_to_write);
+ err = packet_write_gently(fd_out, buf, bytes_to_write);
}
if (!err)
err = packet_flush_gently(fd_out);
-
- ## pkt-line.h ##
-@@
- #include "strbuf.h"
- #include "sideband.h"
-
-+#define LARGE_PACKET_MAX 65520
-+#define LARGE_PACKET_DATA_MAX (LARGE_PACKET_MAX - 4)
-+
-+struct packet_scratch_space {
-+ char buffer[LARGE_PACKET_DATA_MAX]; /* does not include header bytes */
-+};
-+
- /*
- * Write a packetized stream, where each line is preceded by
- * its length (including the header) as a 4-byte hex number.
-@@ pkt-line.h: void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((f
- void packet_buf_write_len(struct strbuf *buf, const char *data, size_t len);
- int packet_flush_gently(int fd);
- int packet_write_fmt_gently(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
--int write_packetized_from_fd(int fd_in, int fd_out);
-+int write_packetized_from_fd(int fd_in, int fd_out, struct packet_scratch_space *scratch);
- int write_packetized_from_buf(const char *src_in, size_t len, int fd_out);
-
- /*
-@@ pkt-line.h: enum packet_read_status packet_reader_read(struct packet_reader *reader);
- enum packet_read_status packet_reader_peek(struct packet_reader *reader);
-
- #define DEFAULT_PACKET_MAX 1000
--#define LARGE_PACKET_MAX 65520
--#define LARGE_PACKET_DATA_MAX (LARGE_PACKET_MAX - 4)
-+
- extern char packet_buffer[LARGE_PACKET_MAX];
++ free(buf);
+ return err;
+ }
- struct packet_writer {
2: 91a9f63d6692 ! 2: 25157c1f4873 pkt-line: do not issue flush packets in write_packetized_*()
@@ Commit message
## convert.c ##
@@ convert.c: static int apply_multi_file_filter(const char *path, const char *src, size_t len
+ goto done;
- if (fd >= 0) {
- struct packet_scratch_space scratch;
-- err = write_packetized_from_fd(fd, process->in, &scratch);
-+ err = write_packetized_from_fd_no_flush(fd, process->in, &scratch);
- } else
+ if (fd >= 0)
+- err = write_packetized_from_fd(fd, process->in);
++ err = write_packetized_from_fd_no_flush(fd, process->in);
+ else
- err = write_packetized_from_buf(src, len, process->in);
+ err = write_packetized_from_buf_no_flush(src, len, process->in);
+ if (err)
@@ pkt-line.c: void packet_buf_write_len(struct strbuf *buf, const char *data, size
packet_trace(data, len, 1);
}
--int write_packetized_from_fd(int fd_in, int fd_out,
-- struct packet_scratch_space *scratch)
-+int write_packetized_from_fd_no_flush(int fd_in, int fd_out,
-+ struct packet_scratch_space *scratch)
+-int write_packetized_from_fd(int fd_in, int fd_out)
++int write_packetized_from_fd_no_flush(int fd_in, int fd_out)
{
+ char *buf = xmalloc(LARGE_PACKET_DATA_MAX);
int err = 0;
- ssize_t bytes_to_write;
-@@ pkt-line.c: int write_packetized_from_fd(int fd_in, int fd_out,
- err = packet_write_gently(fd_out, scratch->buffer,
- bytes_to_write);
+@@ pkt-line.c: int write_packetized_from_fd(int fd_in, int fd_out)
+ break;
+ err = packet_write_gently(fd_out, buf, bytes_to_write);
}
- if (!err)
- err = packet_flush_gently(fd_out);
+ free(buf);
return err;
}
@@ pkt-line.h: void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __at
void packet_buf_write_len(struct strbuf *buf, const char *data, size_t len);
int packet_flush_gently(int fd);
int packet_write_fmt_gently(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
--int write_packetized_from_fd(int fd_in, int fd_out, struct packet_scratch_space *scratch);
+-int write_packetized_from_fd(int fd_in, int fd_out);
-int write_packetized_from_buf(const char *src_in, size_t len, int fd_out);
-+int write_packetized_from_fd_no_flush(int fd_in, int fd_out, struct packet_scratch_space *scratch);
++int write_packetized_from_fd_no_flush(int fd_in, int fd_out);
+int write_packetized_from_buf_no_flush(const char *src_in, size_t len, int fd_out);
/*
3: e05467def4e1 ! 3: af3d13113bc9 pkt-line: (optionally) libify the packet readers
@@ Metadata
Author: Johannes Schindelin <Johannes.Schindelin@gmx.de>
## Commit message ##
- pkt-line: (optionally) libify the packet readers
+ pkt-line: add PACKET_READ_GENTLE_ON_READ_ERROR option
+
+ Introduce PACKET_READ_GENTLE_ON_READ_ERROR option to help libify the
+ packet readers.
So far, the (possibly indirect) callers of `get_packet_data()` can ask
that function to return an error instead of `die()`ing upon end-of-file.
However, random read errors will still cause the process to die.
So let's introduce an explicit option to tell the packet reader
- machinery to please be nice and only return an error.
+ machinery to please be nice and only return an error on read errors.
This change prepares pkt-line for use by long-running daemon processes.
Such processes should be able to serve multiple concurrent clients and
@@ Commit message
a daemon should be able to drop that connection and continue serving
existing and future connections.
- This ability will be used by a Git-aware "Internal FSMonitor" feature
+ This ability will be used by a Git-aware "Builtin FSMonitor" feature
in a later patch series.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
+ Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
## pkt-line.c ##
@@ pkt-line.c: static int get_packet_data(int fd, char **src_buf, size_t *src_size,
@@ pkt-line.c: static int get_packet_data(int fd, char **src_buf, size_t *src_size,
ret = read_in_full(fd, dst, size);
- if (ret < 0)
+ if (ret < 0) {
-+ if (options & PACKET_READ_NEVER_DIE)
++ if (options & PACKET_READ_GENTLE_ON_READ_ERROR)
+ return error_errno(_("read error"));
die_errno(_("read error"));
+ }
@@ pkt-line.c: static int get_packet_data(int fd, char **src_buf, size_t *src_size,
if (options & PACKET_READ_GENTLE_ON_EOF)
return -1;
-+ if (options & PACKET_READ_NEVER_DIE)
++ if (options & PACKET_READ_GENTLE_ON_READ_ERROR)
+ return error(_("the remote end hung up unexpectedly"));
die(_("the remote end hung up unexpectedly"));
}
@@ pkt-line.c: enum packet_read_status packet_read_with_status(int fd, char **src_b
len = packet_length(linelen);
if (len < 0) {
-+ if (options & PACKET_READ_NEVER_DIE)
++ if (options & PACKET_READ_GENTLE_ON_READ_ERROR)
+ return error(_("protocol error: bad line length "
+ "character: %.4s"), linelen);
die(_("protocol error: bad line length character: %.4s"), linelen);
@@ pkt-line.c: enum packet_read_status packet_read_with_status(int fd, char **src_b
*pktlen = 0;
return PACKET_READ_RESPONSE_END;
} else if (len < 4) {
-+ if (options & PACKET_READ_NEVER_DIE)
++ if (options & PACKET_READ_GENTLE_ON_READ_ERROR)
+ return error(_("protocol error: bad line length %d"),
+ len);
die(_("protocol error: bad line length %d"), len);
@@ pkt-line.c: enum packet_read_status packet_read_with_status(int fd, char **src_b
len -= 4;
- if ((unsigned)len >= size)
+ if ((unsigned)len >= size) {
-+ if (options & PACKET_READ_NEVER_DIE)
++ if (options & PACKET_READ_GENTLE_ON_READ_ERROR)
+ return error(_("protocol error: bad line length %d"),
+ len);
die(_("protocol error: bad line length %d"), len);
@@ pkt-line.h: int write_packetized_from_buf_no_flush(const char *src_in, size_t le
* If options contains PACKET_READ_DIE_ON_ERR_PACKET, it dies when it sees an
* ERR packet.
+ *
-+ * With `PACKET_READ_NEVER_DIE`, no errors are allowed to trigger die() (except
-+ * an ERR packet, when `PACKET_READ_DIE_ON_ERR_PACKET` is in effect).
++ * If options contains PACKET_READ_GENTLE_ON_READ_ERROR, we will not die
++ * on read errors, but instead return -1. However, we may still die on an
++ * ERR packet (if requested).
*/
- #define PACKET_READ_GENTLE_ON_EOF (1u<<0)
- #define PACKET_READ_CHOMP_NEWLINE (1u<<1)
- #define PACKET_READ_DIE_ON_ERR_PACKET (1u<<2)
-+#define PACKET_READ_NEVER_DIE (1u<<3)
+-#define PACKET_READ_GENTLE_ON_EOF (1u<<0)
+-#define PACKET_READ_CHOMP_NEWLINE (1u<<1)
+-#define PACKET_READ_DIE_ON_ERR_PACKET (1u<<2)
++#define PACKET_READ_GENTLE_ON_EOF (1u<<0)
++#define PACKET_READ_CHOMP_NEWLINE (1u<<1)
++#define PACKET_READ_DIE_ON_ERR_PACKET (1u<<2)
++#define PACKET_READ_GENTLE_ON_READ_ERROR (1u<<3)
int packet_read(int fd, char **src_buffer, size_t *src_len, char
*buffer, unsigned size, int options);
4: 81e14bed955c = 4: b73e66a69b61 pkt-line: add options argument to read_packetized_to_strbuf()
5: 22eec60761a8 < -: ------------ simple-ipc: design documentation for new IPC mechanism
-: ------------ > 5: 1ae99d824a21 simple-ipc: design documentation for new IPC mechanism
6: 171ec43ecfa4 ! 6: 8b3ce40e4538 simple-ipc: add win32 implementation
@@ compat/simple-ipc/ipc-win32.c (new)
+
+ if (read_packetized_to_strbuf(
+ connection->fd, answer,
-+ PACKET_READ_GENTLE_ON_EOF | PACKET_READ_NEVER_DIE) < 0) {
++ PACKET_READ_GENTLE_ON_EOF | PACKET_READ_GENTLE_ON_READ_ERROR) < 0) {
+ ret = error(_("could not read IPC response"));
+ goto done;
+ }
@@ compat/simple-ipc/ipc-win32.c (new)
+
+ ret = read_packetized_to_strbuf(
+ reply_data.fd, &buf,
-+ PACKET_READ_GENTLE_ON_EOF | PACKET_READ_NEVER_DIE);
++ PACKET_READ_GENTLE_ON_EOF | PACKET_READ_GENTLE_ON_READ_ERROR);
+ if (ret >= 0) {
+ ret = server_thread_data->server_data->application_cb(
+ server_thread_data->server_data->application_data,
@@ compat/simple-ipc/ipc-win32.c (new)
+ *returned_server_data = NULL;
+
+ ret = initialize_pipe_name(path, wpath, ARRAY_SIZE(wpath));
-+ if (ret < 0)
-+ return error(
-+ _("could not create normalized wchar_t path for '%s'"),
-+ path);
++ if (ret < 0) {
++ errno = EINVAL;
++ return -1;
++ }
+
+ hPipeFirst = create_new_pipe(wpath, 1);
-+ if (hPipeFirst == INVALID_HANDLE_VALUE)
-+ return error(_("IPC server already running on '%s'"), path);
++ if (hPipeFirst == INVALID_HANDLE_VALUE) {
++ errno = EADDRINUSE;
++ return -2;
++ }
+
+ server_data = xcalloc(1, sizeof(*server_data));
+ server_data->magic = MAGIC_SERVER_DATA;
@@ simple-ipc.h (new)
+ *
+ * Returns 0 if the asynchronous server pool was started successfully.
+ * Returns -1 if not.
++ * Returns -2 if we could not startup because another server is using
++ * the socket or named pipe.
+ *
+ * When a client IPC message is received, the `application_cb` will be
+ * called (possibly on a random thread) to handle the message and
@@ simple-ipc.h (new)
+ *
+ * Returns 0 after the server has completed successfully.
+ * Returns -1 if the server cannot be started.
++ * Returns -2 if we could not startup because another server is using
++ * the socket or named pipe.
+ *
+ * When a client IPC message is received, the `application_cb` will be
+ * called (possibly on a random thread) to handle the message and
7: b368318e6a23 ! 7: 34df1af98e5b unix-socket: elimiate static unix_stream_socket() helper function
@@ Metadata
Author: Jeff Hostetler <jeffhost@microsoft.com>
## Commit message ##
- unix-socket: elimiate static unix_stream_socket() helper function
+ unix-socket: eliminate static unix_stream_socket() helper function
The static helper function `unix_stream_socket()` calls `die()`. This
is not appropriate for all callers. Eliminate the wrapper function
8: 985b2e02b2df ! 8: d6ff6e0e050a unix-socket: add backlog size option to unix_stream_listen()
@@ builtin/credential-cache--daemon.c: static int serve_cache_loop(int fd)
## unix-socket.c ##
+@@
+ #include "cache.h"
+ #include "unix-socket.h"
+
++#define DEFAULT_UNIX_STREAM_LISTEN_BACKLOG (5)
++
+ static int chdir_len(const char *orig, int len)
+ {
+ char *path = xmemdupz(orig, len);
@@ unix-socket.c: int unix_stream_connect(const char *path)
return -1;
}
@@ unix-socket.h
+ int listen_backlog_size;
+};
+
-+#define DEFAULT_UNIX_STREAM_LISTEN_BACKLOG (5)
-+
-+#define UNIX_STREAM_LISTEN_OPTS_INIT \
-+{ \
-+ .listen_backlog_size = DEFAULT_UNIX_STREAM_LISTEN_BACKLOG, \
-+}
++#define UNIX_STREAM_LISTEN_OPTS_INIT { 0 }
+
int unix_stream_connect(const char *path);
-int unix_stream_listen(const char *path);
9: 1bfa36409d07 ! 9: 21b8d3c63dbf unix-socket: disallow chdir() when creating unix domain sockets
@@ unix-socket.h
+ unsigned int disallow_chdir:1;
};
- #define DEFAULT_UNIX_STREAM_LISTEN_BACKLOG (5)
-@@ unix-socket.h: struct unix_stream_listen_opts {
- #define UNIX_STREAM_LISTEN_OPTS_INIT \
- { \
- .listen_backlog_size = DEFAULT_UNIX_STREAM_LISTEN_BACKLOG, \
-+ .disallow_chdir = 0, \
- }
+ #define UNIX_STREAM_LISTEN_OPTS_INIT { 0 }
-int unix_stream_connect(const char *path);
+int unix_stream_connect(const char *path, int disallow_chdir);
10: b443e11ac32f ! 10: 1ee9de55a106 unix-socket: create `unix_stream_server__listen_with_lock()`
@@ Metadata
Author: Jeff Hostetler <jeffhost@microsoft.com>
## Commit message ##
- unix-socket: create `unix_stream_server__listen_with_lock()`
+ unix-stream-server: create unix domain socket under lock
- Create a version of `unix_stream_listen()` that uses a ".lock" lockfile
- to create the unix domain socket in a race-free manner.
+ Create a wrapper class for `unix_stream_listen()` that uses a ".lock"
+ lockfile to create the unix domain socket in a race-free manner.
Unix domain sockets have a fundamental problem on Unix systems because
they persist in the filesystem until they are deleted. This is
@@ Commit message
As an alternative, we hold a plain lockfile ("<path>.lock") as a
mutual exclusion device. Under the lock, we test if an existing
- socket ("<path>") is has an active server. If not, create a new
- socket and begin listening. Then we rollback the lockfile in all
- cases.
+ socket ("<path>") is has an active server. If not, we create a new
+ socket and begin listening. Then we use "rollback" to delete the
+ lockfile in all cases.
+
+ This wrapper code conceptually exists at a higher-level than the core
+ unix_stream_connect() and unix_stream_listen() routines that it
+ consumes. It is isolated in a wrapper class for clarity.
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
- ## unix-socket.c ##
+ ## Makefile ##
+@@ Makefile: ifdef NO_UNIX_SOCKETS
+ BASIC_CFLAGS += -DNO_UNIX_SOCKETS
+ else
+ LIB_OBJS += unix-socket.o
++ LIB_OBJS += unix-stream-server.o
+ endif
+
+ ifdef USE_WIN32_IPC
+
+ ## contrib/buildsystems/CMakeLists.txt ##
+@@ contrib/buildsystems/CMakeLists.txt: if(CMAKE_SYSTEM_NAME STREQUAL "Windows")
+
+ elseif(CMAKE_SYSTEM_NAME STREQUAL "Linux")
+ add_compile_definitions(PROCFS_EXECUTABLE_PATH="/proc/self/exe" HAVE_DEV_TTY )
+- list(APPEND compat_SOURCES unix-socket.c)
++ list(APPEND compat_SOURCES unix-socket.c unix-stream-server.c)
+ endif()
+
+ if(CMAKE_SYSTEM_NAME STREQUAL "Windows")
+
+ ## unix-stream-server.c (new) ##
@@
- #include "cache.h"
++#include "cache.h"
+#include "lockfile.h"
- #include "unix-socket.h"
-
- static int chdir_len(const char *orig, int len)
-@@ unix-socket.c: int unix_stream_listen(const char *path,
- errno = saved_errno;
- return -1;
- }
++#include "unix-socket.h"
++#include "unix-stream-server.h"
+
++#define DEFAULT_LOCK_TIMEOUT (100)
++
++/*
++ * Try to connect to a unix domain socket at `path` (if it exists) and
++ * see if there is a server listening.
++ *
++ * We don't know if the socket exists, whether a server died and
++ * failed to cleanup, or whether we have a live server listening, so
++ * we "poke" it.
++ *
++ * We immediately hangup without sending/receiving any data because we
++ * don't know anything about the protocol spoken and don't want to
++ * block while writing/reading data. It is sufficient to just know
++ * that someone is listening.
++ */
+static int is_another_server_alive(const char *path,
+ const struct unix_stream_listen_opts *opts)
+{
-+ struct stat st;
-+ int fd;
-+
-+ if (!lstat(path, &st) && S_ISSOCK(st.st_mode)) {
-+ /*
-+ * A socket-inode exists on disk at `path`, but we
-+ * don't know whether it belongs to an active server
-+ * or whether the last server died without cleaning
-+ * up.
-+ *
-+ * Poke it with a trivial connection to try to find
-+ * out.
-+ */
-+ fd = unix_stream_connect(path, opts->disallow_chdir);
-+ if (fd >= 0) {
-+ close(fd);
-+ return 1;
-+ }
++ int fd = unix_stream_connect(path, opts->disallow_chdir);
++ if (fd >= 0) {
++ close(fd);
++ return 1;
+ }
+
+ return 0;
+}
+
-+struct unix_stream_server_socket *unix_stream_server__listen_with_lock(
++int unix_stream_server__create(
+ const char *path,
-+ const struct unix_stream_listen_opts *opts)
++ const struct unix_stream_listen_opts *opts,
++ long timeout_ms,
++ struct unix_stream_server_socket **new_server_socket)
+{
+ struct lock_file lock = LOCK_INIT;
+ int fd_socket;
+ struct unix_stream_server_socket *server_socket;
+
++ *new_server_socket = NULL;
++
++ if (timeout_ms < 0)
++ timeout_ms = DEFAULT_LOCK_TIMEOUT;
++
+ /*
+ * Create a lock at "<path>.lock" if we can.
+ */
-+ if (hold_lock_file_for_update_timeout(&lock, path, 0,
-+ opts->timeout_ms) < 0) {
-+ error_errno(_("could not lock listener socket '%s'"), path);
-+ return NULL;
-+ }
++ if (hold_lock_file_for_update_timeout(&lock, path, 0, timeout_ms) < 0)
++ return -1;
+
+ /*
+ * If another server is listening on "<path>" give up. We do not
+ * want to create a socket and steal future connections from them.
+ */
+ if (is_another_server_alive(path, opts)) {
-+ errno = EADDRINUSE;
-+ error_errno(_("listener socket already in use '%s'"), path);
+ rollback_lock_file(&lock);
-+ return NULL;
++ errno = EADDRINUSE;
++ return -2;
+ }
+
+ /*
@@ unix-socket.c: int unix_stream_listen(const char *path,
+ */
+ fd_socket = unix_stream_listen(path, opts);
+ if (fd_socket < 0) {
-+ error_errno(_("could not create listener socket '%s'"), path);
++ int saved_errno = errno;
+ rollback_lock_file(&lock);
-+ return NULL;
++ errno = saved_errno;
++ return -1;
+ }
+
+ server_socket = xcalloc(1, sizeof(*server_socket));
@@ unix-socket.c: int unix_stream_listen(const char *path,
+ server_socket->fd_socket = fd_socket;
+ lstat(path, &server_socket->st_socket);
+
++ *new_server_socket = server_socket;
++
+ /*
+ * Always rollback (just delete) "<path>.lock" because we already created
+ * "<path>" as a socket and do not want to commit_lock to do the atomic
@@ unix-socket.c: int unix_stream_listen(const char *path,
+ */
+ rollback_lock_file(&lock);
+
-+ return server_socket;
++ return 0;
+}
+
+void unix_stream_server__free(
@@ unix-socket.c: int unix_stream_listen(const char *path,
+
+ if (st_now.st_ino != server_socket->st_socket.st_ino)
+ return 1;
++ if (st_now.st_dev != server_socket->st_socket.st_dev)
++ return 1;
+
-+ /* We might also consider the ctime on some platforms. */
++ if (!S_ISSOCK(st_now.st_mode))
++ return 1;
+
+ return 0;
+}
- ## unix-socket.h ##
+ ## unix-stream-server.h (new) ##
@@
- #define UNIX_SOCKET_H
-
- struct unix_stream_listen_opts {
-+ long timeout_ms;
- int listen_backlog_size;
- unsigned int disallow_chdir:1;
- };
-
-+#define DEFAULT_UNIX_STREAM_LISTEN_TIMEOUT (100)
- #define DEFAULT_UNIX_STREAM_LISTEN_BACKLOG (5)
-
- #define UNIX_STREAM_LISTEN_OPTS_INIT \
- { \
-+ .timeout_ms = DEFAULT_UNIX_STREAM_LISTEN_TIMEOUT, \
- .listen_backlog_size = DEFAULT_UNIX_STREAM_LISTEN_BACKLOG, \
- .disallow_chdir = 0, \
- }
-@@ unix-socket.h: int unix_stream_connect(const char *path, int disallow_chdir);
- int unix_stream_listen(const char *path,
- const struct unix_stream_listen_opts *opts);
-
++#ifndef UNIX_STREAM_SERVER_H
++#define UNIX_STREAM_SERVER_H
++
++#include "unix-socket.h"
++
+struct unix_stream_server_socket {
+ char *path_socket;
+ struct stat st_socket;
@@ unix-socket.h: int unix_stream_connect(const char *path, int disallow_chdir);
+/*
+ * Create a Unix Domain Socket at the given path under the protection
+ * of a '.lock' lockfile.
++ *
++ * Returns 0 on success, -1 on error, -2 if socket is in use.
+ */
-+struct unix_stream_server_socket *unix_stream_server__listen_with_lock(
++int unix_stream_server__create(
+ const char *path,
-+ const struct unix_stream_listen_opts *opts);
++ const struct unix_stream_listen_opts *opts,
++ long timeout_ms,
++ struct unix_stream_server_socket **server_socket);
+
+/*
+ * Close and delete the socket.
@@ unix-socket.h: int unix_stream_connect(const char *path, int disallow_chdir);
+int unix_stream_server__was_stolen(
+ struct unix_stream_server_socket *server_socket);
+
- #endif /* UNIX_SOCKET_H */
++#endif /* UNIX_STREAM_SERVER_H */
11: 43c8db9a4468 ! 11: f2e3b046cc8f simple-ipc: add Unix domain socket implementation
@@ Commit message
## Makefile ##
@@ Makefile: ifdef NO_UNIX_SOCKETS
- BASIC_CFLAGS += -DNO_UNIX_SOCKETS
else
LIB_OBJS += unix-socket.o
+ LIB_OBJS += unix-stream-server.o
+ LIB_OBJS += compat/simple-ipc/ipc-shared.o
+ LIB_OBJS += compat/simple-ipc/ipc-unix-socket.o
endif
@@ compat/simple-ipc/ipc-unix-socket.c (new)
+#include "pkt-line.h"
+#include "thread-utils.h"
+#include "unix-socket.h"
++#include "unix-stream-server.h"
+
+#ifdef NO_UNIX_SOCKETS
+#error compat/simple-ipc/ipc-unix-socket.c requires Unix sockets
@@ compat/simple-ipc/ipc-unix-socket.c (new)
+
+ if (read_packetized_to_strbuf(
+ connection->fd, answer,
-+ PACKET_READ_GENTLE_ON_EOF | PACKET_READ_NEVER_DIE) < 0) {
++ PACKET_READ_GENTLE_ON_EOF | PACKET_READ_GENTLE_ON_READ_ERROR) < 0) {
+ ret = error(_("could not read IPC response"));
+ goto done;
+ }
@@ compat/simple-ipc/ipc-unix-socket.c (new)
+
+ ret = read_packetized_to_strbuf(
+ reply_data.fd, &buf,
-+ PACKET_READ_GENTLE_ON_EOF | PACKET_READ_NEVER_DIE);
++ PACKET_READ_GENTLE_ON_EOF | PACKET_READ_GENTLE_ON_READ_ERROR);
+ if (ret >= 0) {
+ ret = worker_thread_data->server_data->application_cb(
+ worker_thread_data->server_data->application_data,
@@ compat/simple-ipc/ipc-unix-socket.c (new)
+ */
+#define LISTEN_BACKLOG (50)
+
-+static struct unix_stream_server_socket *create_listener_socket(
++static int create_listener_socket(
+ const char *path,
-+ const struct ipc_server_opts *ipc_opts)
++ const struct ipc_server_opts *ipc_opts,
++ struct unix_stream_server_socket **new_server_socket)
+{
+ struct unix_stream_server_socket *server_socket = NULL;
+ struct unix_stream_listen_opts uslg_opts = UNIX_STREAM_LISTEN_OPTS_INIT;
++ int ret;
+
+ uslg_opts.listen_backlog_size = LISTEN_BACKLOG;
+ uslg_opts.disallow_chdir = ipc_opts->uds_disallow_chdir;
+
-+ server_socket = unix_stream_server__listen_with_lock(path, &uslg_opts);
-+ if (!server_socket)
-+ return NULL;
++ ret = unix_stream_server__create(path, &uslg_opts, -1, &server_socket);
++ if (ret)
++ return ret;
+
+ if (set_socket_blocking_flag(server_socket->fd_socket, 1)) {
+ int saved_errno = errno;
-+ error_errno(_("could not set listener socket nonblocking '%s'"),
-+ path);
+ unix_stream_server__free(server_socket);
+ errno = saved_errno;
-+ return NULL;
++ return -1;
+ }
+
++ *new_server_socket = server_socket;
++
+ trace2_data_string("ipc-server", NULL, "listen-with-lock", path);
-+ return server_socket;
++ return 0;
+}
+
-+static struct unix_stream_server_socket *setup_listener_socket(
++static int setup_listener_socket(
+ const char *path,
-+ const struct ipc_server_opts *ipc_opts)
++ const struct ipc_server_opts *ipc_opts,
++ struct unix_stream_server_socket **new_server_socket)
+{
-+ struct unix_stream_server_socket *server_socket;
++ int ret, saved_errno;
+
+ trace2_region_enter("ipc-server", "create-listener_socket", NULL);
-+ server_socket = create_listener_socket(path, ipc_opts);
++
++ ret = create_listener_socket(path, ipc_opts, new_server_socket);
++
++ saved_errno = errno;
+ trace2_region_leave("ipc-server", "create-listener_socket", NULL);
++ errno = saved_errno;
+
-+ return server_socket;
++ return ret;
+}
+
+/*
@@ compat/simple-ipc/ipc-unix-socket.c (new)
+ struct ipc_server_data *server_data;
+ int sv[2];
+ int k;
++ int ret;
+ int nr_threads = opts->nr_threads;
+
+ *returned_server_data = NULL;
@@ compat/simple-ipc/ipc-unix-socket.c (new)
+ * connection or a shutdown request without spinning.
+ */
+ if (socketpair(AF_UNIX, SOCK_STREAM, 0, sv) < 0)
-+ return error_errno(_("could not create socketpair for '%s'"),
-+ path);
++ return -1;
+
+ if (set_socket_blocking_flag(sv[1], 1)) {
+ int saved_errno = errno;
+ close(sv[0]);
+ close(sv[1]);
+ errno = saved_errno;
-+ return error_errno(_("making socketpair nonblocking '%s'"),
-+ path);
++ return -1;
+ }
+
-+ server_socket = setup_listener_socket(path, opts);
-+ if (!server_socket) {
++ ret = setup_listener_socket(path, opts, &server_socket);
++ if (ret) {
+ int saved_errno = errno;
+ close(sv[0]);
+ close(sv[1]);
+ errno = saved_errno;
-+ return -1;
++ return ret;
+ }
+
+ server_data = xcalloc(1, sizeof(*server_data));
12: 09568a6500dd ! 12: 6ccc7472096f t0052: add simple-ipc tests and t/helper/test-simple-ipc tool
@@ t/helper/test-simple-ipc.c (new)
+ return app__unhandled_command(command, reply_cb, reply_data);
+}
+
++struct cl_args
++{
++ const char *subcommand;
++ const char *path;
++ const char *token;
++
++ int nr_threads;
++ int max_wait_sec;
++ int bytecount;
++ int batchsize;
++
++ char bytevalue;
++};
++
++static struct cl_args cl_args = {
++ .subcommand = NULL,
++ .path = "ipc-test",
++ .token = NULL,
++
++ .nr_threads = 5,
++ .max_wait_sec = 60,
++ .bytecount = 1024,
++ .batchsize = 10,
++
++ .bytevalue = 'x',
++};
++
+/*
+ * This process will run as a simple-ipc server and listen for IPC commands
+ * from client processes.
+ */
-+static int daemon__run_server(const char *path, int argc, const char **argv)
++static int daemon__run_server(void)
+{
-+ struct ipc_server_opts opts = {
-+ .nr_threads = 5
-+ };
++ int ret;
+
-+ const char * const daemon_usage[] = {
-+ N_("test-helper simple-ipc run-daemon [<options>"),
-+ NULL
-+ };
-+ struct option daemon_options[] = {
-+ OPT_INTEGER(0, "threads", &opts.nr_threads,
-+ N_("number of threads in server thread pool")),
-+ OPT_END()
++ struct ipc_server_opts opts = {
++ .nr_threads = cl_args.nr_threads,
+ };
+
-+ argc = parse_options(argc, argv, NULL, daemon_options, daemon_usage, 0);
-+
-+ if (opts.nr_threads < 1)
-+ opts.nr_threads = 1;
-+
+ /*
+ * Synchronously run the ipc-server. We don't need any application
+ * instance data, so pass an arbitrary pointer (that we'll later
+ * verify made the round trip).
+ */
-+ return ipc_server_run(path, &opts, test_app_cb, (void*)&my_app_data);
++ ret = ipc_server_run(cl_args.path, &opts, test_app_cb, (void*)&my_app_data);
++ if (ret == -2)
++ error(_("socket/pipe already in use: '%s'"), cl_args.path);
++ else if (ret == -1)
++ error_errno(_("could not start server on: '%s'"), cl_args.path);
++
++ return ret;
+}
+
+#ifndef GIT_WINDOWS_NATIVE
@@ t/helper/test-simple-ipc.c (new)
+ * This is adapted from `daemonize()`. Use `fork()` to directly create and
+ * run the daemon in a child process.
+ */
-+static int spawn_server(const char *path,
-+ const struct ipc_server_opts *opts,
-+ pid_t *pid)
++static int spawn_server(pid_t *pid)
+{
++ struct ipc_server_opts opts = {
++ .nr_threads = cl_args.nr_threads,
++ };
++
+ *pid = fork();
+
+ switch (*pid) {
@@ t/helper/test-simple-ipc.c (new)
+ close(2);
+ sanitize_stdfds();
+
-+ return ipc_server_run(path, opts, test_app_cb, (void*)&my_app_data);
++ return ipc_server_run(cl_args.path, &opts, test_app_cb,
++ (void*)&my_app_data);
+
+ case -1:
+ return error_errno(_("could not spawn daemon in the background"));
@@ t/helper/test-simple-ipc.c (new)
+ * have `fork(2)`. Spawn a normal Windows child process but without the
+ * limitations of `start_command()` and `finish_command()`.
+ */
-+static int spawn_server(const char *path,
-+ const struct ipc_server_opts *opts,
-+ pid_t *pid)
++static int spawn_server(pid_t *pid)
+{
+ char test_tool_exe[MAX_PATH];
+ struct strvec args = STRVEC_INIT;
@@ t/helper/test-simple-ipc.c (new)
+ strvec_push(&args, test_tool_exe);
+ strvec_push(&args, "simple-ipc");
+ strvec_push(&args, "run-daemon");
-+ strvec_pushf(&args, "--threads=%d", opts->nr_threads);
++ strvec_pushf(&args, "--name=%s", cl_args.path);
++ strvec_pushf(&args, "--threads=%d", cl_args.nr_threads);
+
+ *pid = mingw_spawnvpe(args.v[0], args.v, NULL, NULL, in, out, out);
+ close(in);
@@ t/helper/test-simple-ipc.c (new)
+ * let it get started and begin listening for requests on the socket
+ * before reporting our success.
+ */
-+static int wait_for_server_startup(const char * path, pid_t pid_child,
-+ int max_wait_sec)
++static int wait_for_server_startup(pid_t pid_child)
+{
+ int status;
+ pid_t pid_seen;
@@ t/helper/test-simple-ipc.c (new)
+ time_t time_limit, now;
+
+ time(&time_limit);
-+ time_limit += max_wait_sec;
++ time_limit += cl_args.max_wait_sec;
+
+ for (;;) {
+ pid_seen = waitpid(pid_child, &status, WNOHANG);
@@ t/helper/test-simple-ipc.c (new)
+ * after a timeout on the lock), but we don't
+ * care (who responds) if the socket is live.
+ */
-+ s = ipc_get_active_state(path);
++ s = ipc_get_active_state(cl_args.path);
+ if (s == IPC_STATE__LISTENING)
+ return 0;
+
@@ t/helper/test-simple-ipc.c (new)
+ *
+ * Again, we don't care who services the socket.
+ */
-+ s = ipc_get_active_state(path);
++ s = ipc_get_active_state(cl_args.path);
+ if (s == IPC_STATE__LISTENING)
+ return 0;
+
@@ t/helper/test-simple-ipc.c (new)
+ * more control and better error reporting (and makes it easier to write
+ * unit tests).
+ */
-+static int daemon__start_server(const char *path, int argc, const char **argv)
++static int daemon__start_server(void)
+{
+ pid_t pid_child;
+ int ret;
-+ int max_wait_sec = 60;
-+ struct ipc_server_opts opts = {
-+ .nr_threads = 5
-+ };
-+
-+ const char * const daemon_usage[] = {
-+ N_("test-helper simple-ipc start-daemon [<options>"),
-+ NULL
-+ };
-+
-+ struct option daemon_options[] = {
-+ OPT_INTEGER(0, "max-wait", &max_wait_sec,
-+ N_("seconds to wait for daemon to startup")),
-+ OPT_INTEGER(0, "threads", &opts.nr_threads,
-+ N_("number of threads in server thread pool")),
-+ OPT_END()
-+ };
-+
-+ argc = parse_options(argc, argv, NULL, daemon_options, daemon_usage, 0);
-+
-+ if (max_wait_sec < 0)
-+ max_wait_sec = 0;
-+ if (opts.nr_threads < 1)
-+ opts.nr_threads = 1;
+
+ /*
+ * Run the actual daemon in a background process.
+ */
-+ ret = spawn_server(path, &opts, &pid_child);
++ ret = spawn_server(&pid_child);
+ if (pid_child <= 0)
+ return ret;
+
@@ t/helper/test-simple-ipc.c (new)
+ * Let the parent wait for the child process to get started
+ * and begin listening for requests on the socket.
+ */
-+ ret = wait_for_server_startup(path, pid_child, max_wait_sec);
++ ret = wait_for_server_startup(pid_child);
+
+ return ret;
+}
@@ t/helper/test-simple-ipc.c (new)
+ *
+ * Returns 0 if the server is alive.
+ */
-+static int client__probe_server(const char *path)
++static int client__probe_server(void)
+{
+ enum ipc_active_state s;
+
-+ s = ipc_get_active_state(path);
++ s = ipc_get_active_state(cl_args.path);
+ switch (s) {
+ case IPC_STATE__LISTENING:
+ return 0;
+
+ case IPC_STATE__NOT_LISTENING:
-+ return error("no server listening at '%s'", path);
++ return error("no server listening at '%s'", cl_args.path);
+
+ case IPC_STATE__PATH_NOT_FOUND:
-+ return error("path not found '%s'", path);
++ return error("path not found '%s'", cl_args.path);
+
+ case IPC_STATE__INVALID_PATH:
-+ return error("invalid pipe/socket name '%s'", path);
++ return error("invalid pipe/socket name '%s'", cl_args.path);
+
+ case IPC_STATE__OTHER_ERROR:
+ default:
-+ return error("other error for '%s'", path);
++ return error("other error for '%s'", cl_args.path);
+ }
+}
+
+/*
-+ * Send an IPC command to an already-running server daemon and print the
-+ * response.
++ * Send an IPC command token to an already-running server daemon and
++ * print the response.
+ *
-+ * argv[2] contains a simple (1 word) command that `test_app_cb()` (in
-+ * the daemon process) will understand.
++ * This is a simple 1 word command/token that `test_app_cb()` (in the
++ * daemon process) will understand.
+ */
-+static int client__send_ipc(int argc, const char **argv, const char *path)
++static int client__send_ipc(void)
+{
-+ const char *command = argc > 2 ? argv[2] : "(no command)";
++ const char *command = "(no-command)";
+ struct strbuf buf = STRBUF_INIT;
+ struct ipc_client_connect_options options
+ = IPC_CLIENT_CONNECT_OPTIONS_INIT;
+
++ if (cl_args.token && *cl_args.token)
++ command = cl_args.token;
++
+ options.wait_if_busy = 1;
+ options.wait_if_not_found = 0;
+
-+ if (!ipc_client_send_command(path, &options, command, &buf)) {
++ if (!ipc_client_send_command(cl_args.path, &options, command, &buf)) {
+ if (buf.len) {
+ printf("%s\n", buf.buf);
+ fflush(stdout);
@@ t/helper/test-simple-ipc.c (new)
+ return 0;
+ }
+
-+ return error("failed to send '%s' to '%s'", command, path);
++ return error("failed to send '%s' to '%s'", command, cl_args.path);
+}
+
+/*
@@ t/helper/test-simple-ipc.c (new)
+ * event in the server, so we spin and wait here for it to actually
+ * shutdown to make the unit tests a little easier to write.
+ */
-+static int client__stop_server(int argc, const char **argv, const char *path)
++static int client__stop_server(void)
+{
-+ const char *send_quit[] = { argv[0], "send", "quit", NULL };
-+ int max_wait_sec = 60;
+ int ret;
+ time_t time_limit, now;
+ enum ipc_active_state s;
+
-+ const char * const stop_usage[] = {
-+ N_("test-helper simple-ipc stop-daemon [<options>]"),
-+ NULL
-+ };
-+
-+ struct option stop_options[] = {
-+ OPT_INTEGER(0, "max-wait", &max_wait_sec,
-+ N_("seconds to wait for daemon to stop")),
-+ OPT_END()
-+ };
-+
-+ argc = parse_options(argc, argv, NULL, stop_options, stop_usage, 0);
-+
-+ if (max_wait_sec < 0)
-+ max_wait_sec = 0;
-+
+ time(&time_limit);
-+ time_limit += max_wait_sec;
++ time_limit += cl_args.max_wait_sec;
++
++ cl_args.token = "quit";
+
-+ ret = client__send_ipc(3, send_quit, path);
++ ret = client__send_ipc();
+ if (ret)
+ return ret;
+
+ for (;;) {
+ sleep_millisec(100);
+
-+ s = ipc_get_active_state(path);
++ s = ipc_get_active_state(cl_args.path);
+
+ if (s != IPC_STATE__LISTENING) {
+ /*
@@ t/helper/test-simple-ipc.c (new)
+/*
+ * Send an IPC command with ballast to an already-running server daemon.
+ */
-+static int client__sendbytes(int argc, const char **argv, const char *path)
++static int client__sendbytes(void)
+{
-+ int bytecount = 1024;
-+ char *string = "x";
-+ const char * const sendbytes_usage[] = {
-+ N_("test-helper simple-ipc sendbytes [<options>]"),
-+ NULL
-+ };
-+ struct option sendbytes_options[] = {
-+ OPT_INTEGER(0, "bytecount", &bytecount, N_("number of bytes")),
-+ OPT_STRING(0, "byte", &string, N_("byte"), N_("ballast")),
-+ OPT_END()
-+ };
+ struct ipc_client_connect_options options
+ = IPC_CLIENT_CONNECT_OPTIONS_INIT;
+
@@ t/helper/test-simple-ipc.c (new)
+ options.wait_if_not_found = 0;
+ options.uds_disallow_chdir = 0;
+
-+ argc = parse_options(argc, argv, NULL, sendbytes_options, sendbytes_usage, 0);
-+
-+ return do_sendbytes(bytecount, string[0], path, &options);
++ return do_sendbytes(cl_args.bytecount, cl_args.bytevalue, cl_args.path,
++ &options);
+}
+
+struct multiple_thread_data {
@@ t/helper/test-simple-ipc.c (new)
+ * Start a client-side thread pool. Each thread sends a series of
+ * IPC requests. Each request is on a new connection to the server.
+ */
-+static int client__multiple(int argc, const char **argv, const char *path)
++static int client__multiple(void)
+{
+ struct multiple_thread_data *list = NULL;
+ int k;
-+ int nr_threads = 5;
-+ int bytecount = 1;
-+ int batchsize = 10;
+ int sum_join_errors = 0;
+ int sum_thread_errors = 0;
+ int sum_good = 0;
+
-+ const char * const multiple_usage[] = {
-+ N_("test-helper simple-ipc multiple [<options>]"),
-+ NULL
-+ };
-+ struct option multiple_options[] = {
-+ OPT_INTEGER(0, "bytecount", &bytecount, N_("number of bytes")),
-+ OPT_INTEGER(0, "threads", &nr_threads, N_("number of threads")),
-+ OPT_INTEGER(0, "batchsize", &batchsize, N_("number of requests per thread")),
-+ OPT_END()
-+ };
-+
-+ argc = parse_options(argc, argv, NULL, multiple_options, multiple_usage, 0);
-+
-+ if (bytecount < 1)
-+ bytecount = 1;
-+ if (nr_threads < 1)
-+ nr_threads = 1;
-+ if (batchsize < 1)
-+ batchsize = 1;
-+
-+ for (k = 0; k < nr_threads; k++) {
++ for (k = 0; k < cl_args.nr_threads; k++) {
+ struct multiple_thread_data *d = xcalloc(1, sizeof(*d));
+ d->next = list;
-+ d->path = path;
-+ d->bytecount = bytecount + batchsize*(k/26);
-+ d->batchsize = batchsize;
++ d->path = cl_args.path;
++ d->bytecount = cl_args.bytecount + cl_args.batchsize*(k/26);
++ d->batchsize = cl_args.batchsize;
+ d->sum_errors = 0;
+ d->sum_good = 0;
+ d->letter = 'A' + (k % 26);
@@ t/helper/test-simple-ipc.c (new)
+
+int cmd__simple_ipc(int argc, const char **argv)
+{
-+ const char *path = "ipc-test";
++ const char * const simple_ipc_usage[] = {
++ N_("test-helper simple-ipc is-active [<name>] [<options>]"),
++ N_("test-helper simple-ipc run-daemon [<name>] [<threads>]"),
++ N_("test-helper simple-ipc start-daemon [<name>] [<threads>] [<max-wait>]"),
++ N_("test-helper simple-ipc stop-daemon [<name>] [<max-wait>]"),
++ N_("test-helper simple-ipc send [<name>] [<token>]"),
++ N_("test-helper simple-ipc sendbytes [<name>] [<bytecount>] [<byte>]"),
++ N_("test-helper simple-ipc multiple [<name>] [<threads>] [<bytecount>] [<batchsize>]"),
++ NULL
++ };
++
++ const char *bytevalue = NULL;
++
++ struct option options[] = {
++#ifndef GIT_WINDOWS_NATIVE
++ OPT_STRING(0, "name", &cl_args.path, N_("name"), N_("name or pathname of unix domain socket")),
++#else
++ OPT_STRING(0, "name", &cl_args.path, N_("name"), N_("named-pipe name")),
++#endif
++ OPT_INTEGER(0, "threads", &cl_args.nr_threads, N_("number of threads in server thread pool")),
++ OPT_INTEGER(0, "max-wait", &cl_args.max_wait_sec, N_("seconds to wait for daemon to start or stop")),
++
++ OPT_INTEGER(0, "bytecount", &cl_args.bytecount, N_("number of bytes")),
++ OPT_INTEGER(0, "batchsize", &cl_args.batchsize, N_("number of requests per thread")),
++
++ OPT_STRING(0, "byte", &bytevalue, N_("byte"), N_("ballast character")),
++ OPT_STRING(0, "token", &cl_args.token, N_("token"), N_("command token to send to the server")),
++
++ OPT_END()
++ };
++
++ if (argc < 2)
++ usage_with_options(simple_ipc_usage, options);
++
++ if (argc == 2 && !strcmp(argv[1], "-h"))
++ usage_with_options(simple_ipc_usage, options);
+
+ if (argc == 2 && !strcmp(argv[1], "SUPPORTS_SIMPLE_IPC"))
+ return 0;
+
++ cl_args.subcommand = argv[1];
++
++ argc--;
++ argv++;
++
++ argc = parse_options(argc, argv, NULL, options, simple_ipc_usage, 0);
++
++ if (cl_args.nr_threads < 1)
++ cl_args.nr_threads = 1;
++ if (cl_args.max_wait_sec < 0)
++ cl_args.max_wait_sec = 0;
++ if (cl_args.bytecount < 1)
++ cl_args.bytecount = 1;
++ if (cl_args.batchsize < 1)
++ cl_args.batchsize = 1;
++
++ if (bytevalue && *bytevalue)
++ cl_args.bytevalue = bytevalue[0];
++
+ /*
+ * Use '!!' on all dispatch functions to map from `error()` style
+ * (returns -1) style to `test_must_fail` style (expects 1). This
+ * makes shell error messages less confusing.
+ */
+
-+ if (argc == 2 && !strcmp(argv[1], "is-active"))
-+ return !!client__probe_server(path);
++ if (!strcmp(cl_args.subcommand, "is-active"))
++ return !!client__probe_server();
+
-+ if (argc >= 2 && !strcmp(argv[1], "run-daemon"))
-+ return !!daemon__run_server(path, argc, argv);
++ if (!strcmp(cl_args.subcommand, "run-daemon"))
++ return !!daemon__run_server();
+
-+ if (argc >= 2 && !strcmp(argv[1], "start-daemon"))
-+ return !!daemon__start_server(path, argc, argv);
++ if (!strcmp(cl_args.subcommand, "start-daemon"))
++ return !!daemon__start_server();
+
+ /*
+ * Client commands follow. Ensure a server is running before
-+ * going any further.
++ * sending any data. This might be overkill, but then again
++ * this is a test harness.
+ */
-+ if (client__probe_server(path))
-+ return 1;
+
-+ if (argc >= 2 && !strcmp(argv[1], "stop-daemon"))
-+ return !!client__stop_server(argc, argv, path);
++ if (!strcmp(cl_args.subcommand, "stop-daemon")) {
++ if (client__probe_server())
++ return 1;
++ return !!client__stop_server();
++ }
+
-+ if ((argc == 2 || argc == 3) && !strcmp(argv[1], "send"))
-+ return !!client__send_ipc(argc, argv, path);
++ if (!strcmp(cl_args.subcommand, "send")) {
++ if (client__probe_server())
++ return 1;
++ return !!client__send_ipc();
++ }
+
-+ if (argc >= 2 && !strcmp(argv[1], "sendbytes"))
-+ return !!client__sendbytes(argc, argv, path);
++ if (!strcmp(cl_args.subcommand, "sendbytes")) {
++ if (client__probe_server())
++ return 1;
++ return !!client__sendbytes();
++ }
+
-+ if (argc >= 2 && !strcmp(argv[1], "multiple"))
-+ return !!client__multiple(argc, argv, path);
++ if (!strcmp(cl_args.subcommand, "multiple")) {
++ if (client__probe_server())
++ return 1;
++ return !!client__multiple();
++ }
+
-+ die("Unhandled argv[1]: '%s'", argv[1]);
++ die("Unhandled subcommand: '%s'", cl_args.subcommand);
+}
+#endif
@@ t/t0052-simple-ipc.sh (new)
+'
+
+test_expect_success 'simple command server' '
-+ test-tool simple-ipc send ping >actual &&
++ test-tool simple-ipc send --token=ping >actual &&
+ echo pong >expect &&
+ test_cmp expect actual
+'
@@ t/t0052-simple-ipc.sh (new)
+'
+
+test_expect_success 'big response' '
-+ test-tool simple-ipc send big >actual &&
++ test-tool simple-ipc send --token=big >actual &&
+ test_line_count -ge 10000 actual &&
+ grep -q "big: [0]*9999\$" actual
+'
+
+test_expect_success 'chunk response' '
-+ test-tool simple-ipc send chunk >actual &&
++ test-tool simple-ipc send --token=chunk >actual &&
+ test_line_count -ge 10000 actual &&
+ grep -q "big: [0]*9999\$" actual
+'
+
+test_expect_success 'slow response' '
-+ test-tool simple-ipc send slow >actual &&
++ test-tool simple-ipc send --token=slow >actual &&
+ test_line_count -ge 100 actual &&
+ grep -q "big: [0]*99\$" actual
+'
@@ t/t0052-simple-ipc.sh (new)
+test_expect_success 'stop-daemon works' '
+ test-tool simple-ipc stop-daemon &&
+ test_must_fail test-tool simple-ipc is-active &&
-+ test_must_fail test-tool simple-ipc send ping
++ test_must_fail test-tool simple-ipc send --token=ping
+'
+
+test_done-- gitgitgadget