From b482b9e069022957cddaca24b72975fd4cfc462a Mon Sep 17 00:00:00 2001 From: Scott Mcdermott Date: Wed, 10 Apr 2024 06:03:44 -0700 Subject: [PATCH 1/2] [interlink] do not nest retries for connect_to_af_unix(), it has already Not sure why a retry loop using MAX_BIND_TRIES / BIND_TRIES_DELAY surrounds connect_to_af_unix(), since bind() is done elsewhere in the code, and connect_to_af_unix() already has its own retry loop inside the function, with its own set of defines for retries, i.e. MAX_CONNECT_TRIES / CONNECT_TRIES_DELAY, surrounding the connect(). No need to have two levels of retry; if it's not successful within connect_to_af_unix() it will never be. --- src/main/interlink.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/main/interlink.c b/src/main/interlink.c index 01b834f3..50e05401 100644 --- a/src/main/interlink.c +++ b/src/main/interlink.c @@ -530,19 +530,10 @@ init_interlink(void) if (pid == -1) return -1; if (pid > 0) { - int i; - - for (i = 1; i <= (MAX_BIND_TRIES+2); ++i) { - fd = connect_to_af_unix(); - - if (fd != -1) { - master_pid = pid; - return fd; - } - elinks_usleep(BIND_TRIES_DELAY * i); - } - return -1; + master_pid = pid; + return connect_to_af_unix(); } + /* child */ master_pid = getpid(); close_terminal_pipes(); From 3d1f020a3b78b4842865e6976b9e458e74732630 Mon Sep 17 00:00:00 2001 From: Scott Mcdermott Date: Fri, 12 Apr 2024 22:51:01 -0700 Subject: [PATCH 2/2] [interlink] speed up first elinks, and use pipe trigger for fork_on_start When the first elinks session starts (whether fork_on_start or not), it tries to connect on AF_UNIX socket, to see if an existing master is present. The current code implements a retry loop with linear-increase retry backoff, 50ms *= attempts, with 3 attempts, total delay 300ms, before concluding master is not present. After forking for fork_on_start, we do have to wait for the new master to finish setup, so we can connect to it. This is the case where retry loop might be useful. However in fork_on_start=0 case, there should be no reason to ever wait: if we get ECONNREFUSED or ENOENT, the master simply is not there, we can go on about our business as the master. With fork_on_start=1 however, we will race with the child (that will become the new master), which has to complete its socket/bind/listen sequence. Therefore we typically have to wait another 50ms (first attempt delay), for a total of 350ms delay. In both cases, there does not seem to be a purpose to the initial connection attempt retry. Conclusion after connect() failure should be authoritative: there is no master. We do not race with anyone. If we have done fork_on_start and have to wait, we can instead open a pipe before forking, and wait for the "new" master (its child) to send us a byte over the pipe. Thus, we do not need to poll, but can simply block until we get the trigger, to indicate that AF_UNIX socket setup has completed. This speeds up the first start of elinks by 300ms for fork_on_start=0, and between 300-350ms for fork_on_start=1 (or possibly more, if the machine is loaded and child did not finish AF_UNIX setup in first 50ms). --- src/main/interlink.c | 73 ++++++++++++++++++++++++++++---------------- 1 file changed, 47 insertions(+), 26 deletions(-) diff --git a/src/main/interlink.c b/src/main/interlink.c index 50e05401..c7b73e4f 100644 --- a/src/main/interlink.c +++ b/src/main/interlink.c @@ -321,11 +321,6 @@ setsock_reuse_addr(int fd) /* Number of connections in listen backlog. */ #define LISTEN_BACKLOG 100 -/* Max. number of connect attempts. */ -#define MAX_CONNECT_TRIES 3 -/* Base delay in useconds between connect attempts. */ -#define CONNECT_TRIES_DELAY 50000 - static void report_af_unix_error(const char *function, int error) { @@ -443,31 +438,24 @@ free_and_error: static int connect_to_af_unix(void) { - int attempts = 0; int pf = get_address(&s_info_connect, ADDR_IP_CLIENT); - while (pf != -1 && attempts++ < MAX_CONNECT_TRIES) { - int saved_errno; - + if (pf != -1) { s_info_connect.fd = socket(pf, SOCK_STREAM, 0); if (s_info_connect.fd == -1) { report_af_unix_error("socket()", errno); - break; - } - - if (connect(s_info_connect.fd, s_info_connect.addr, - s_info_connect.size) >= 0) + } else { + if (connect(s_info_connect.fd, s_info_connect.addr, + s_info_connect.size) >= 0) { return s_info_connect.fd; - - saved_errno = errno; - close(s_info_connect.fd); - - if (saved_errno != ECONNREFUSED && saved_errno != ENOENT) { - report_af_unix_error("connect()", errno); - break; + } + if (errno != ECONNREFUSED && errno != ENOENT) { + report_af_unix_error("connect()", errno); + } + else if (close(s_info_connect.fd) == -1) { + report_af_unix_error("close(afsock)", errno); + } } - - elinks_usleep(CONNECT_TRIES_DELAY * attempts); } mem_free_set(&s_info_connect.addr, NULL); @@ -517,6 +505,9 @@ done_interlink(void) int init_interlink(void) { + int pipefds[2]; + char trigger; + ssize_t n; int fd = connect_to_af_unix(); if (fd != -1 || remote_session_flags) return fd; @@ -525,12 +516,33 @@ init_interlink(void) if (get_opt_bool("ui.sessions.fork_on_start", NULL)) { - pid_t pid = fork(); + pid_t pid; + if (pipe(pipefds) == -1) { + report_af_unix_error("pipe()", errno); + return -1; + } + pid = fork(); if (pid == -1) return -1; if (pid > 0) { + int error = 0; master_pid = pid; + + /* wait for forked child to complete the bind() */ + if ((n = read(pipefds[0], &trigger, 1)) <= 0) { + if (n == 0) { + errno = EPIPE; /* write end closed */ + } + report_af_unix_error("read()", errno); + error = 1; + } + if (close(pipefds[0]) == -1) { + report_af_unix_error("close(pipe_r)", errno); + error = 1; + } + if (error) return -1; + return connect_to_af_unix(); } @@ -539,6 +551,17 @@ init_interlink(void) close_terminal_pipes(); } bind_to_af_unix(); + if (get_opt_bool("ui.sessions.fork_on_start", NULL)) { + if ((n = write(pipefds[1], &trigger, 1)) <= 0) { + if (n == 0) { + errno = EPIPE; /* read end closed */ + } + report_af_unix_error("write()", errno); + } + if (close(pipefds[1]) == -1) { + report_af_unix_error("close(pipe_w)", errno); + } + } return -1; } @@ -546,5 +569,3 @@ init_interlink(void) #undef MAX_BIND_TRIES #undef BIND_TRIES_DELAY #undef LISTEN_BACKLOG -#undef MAX_CONNECT_TRIES -#undef CONNECT_TRIES_DELAY