[v5,00/12] Simple IPC Mechanism

Message ID	pull.766.v5.git.1615302157.gitgitgadget@gmail.com (mailing list archive)
Headers	show Return-Path: <git-owner@kernel.org> Message-Id: <pull.766.v5.git.1615302157.gitgitgadget@gmail.com> In-Reply-To: <pull.766.v4.git.1613598529.gitgitgadget@gmail.com> References: <pull.766.v4.git.1613598529.gitgitgadget@gmail.com> From: "Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> Date: Tue, 09 Mar 2021 15:02:25 +0000 Subject: [PATCH v5 00/12] Simple IPC Mechanism Fcc: Sent Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MIME-Version: 1.0 To: git@vger.kernel.org Cc: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason <avarab@gmail.com>, Jeff Hostetler <git@jeffhostetler.com>, Jeff King <peff@peff.net>, SZEDER =?utf-8?b?R8OhYm9y?= <szeder.dev@gmail.com>, Johannes Schindelin <Johannes.Schindelin@gmx.de>, Chris Torek <chris.torek@gmail.com>, Jeff Hostetler <jeffhost@microsoft.com> Precedence: bulk
Series	Simple IPC Mechanism \| expand [v5,00/12] Simple IPC Mechanism [v5,01/12] pkt-line: eliminate the need for static buffer in packet_write_gently() [v5,02/12] pkt-line: do not issue flush packets in write_packetized_*() [v5,03/12] pkt-line: add PACKET_READ_GENTLE_ON_READ_ERROR option [v5,04/12] pkt-line: add options argument to read_packetized_to_strbuf() [v5,05/12] simple-ipc: design documentation for new IPC mechanism [v5,06/12] simple-ipc: add win32 implementation [v5,07/12] unix-socket: eliminate static unix_stream_socket() helper function [v5,08/12] unix-socket: add backlog size option to unix_stream_listen() [v5,09/12] unix-socket: disallow chdir() when creating unix domain sockets [v5,10/12] unix-stream-server: create unix domain socket under lock [v5,11/12] simple-ipc: add Unix domain socket implementation [v5,12/12] t0052: add simple-ipc tests and t/helper/test-simple-ipc tool

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 tool Johannes 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

[v5,00/12] Simple IPC Mechanism

Message

Comments