From: Stefan Sperling Subject: repo disconect cleanup To: gameoftrees@openbsd.org Date: Thu, 9 Feb 2023 17:47:25 +0100 Some follow-up tweaks related to the gotd segfault fix from commit 90270f794c7f4e1ce2b58c8b5bfdc190df4799b6 ----------------------------------------------- do not expect to see a DISCONNECT message in repo processes The parent no longer sends this message. Perform related cleanup in the shutdown path instead. diff 90270f794c7f4e1ce2b58c8b5bfdc190df4799b6 94668037c826c7562bb0b036b9355a7bd88185b4 commit - 90270f794c7f4e1ce2b58c8b5bfdc190df4799b6 commit + 94668037c826c7562bb0b036b9355a7bd88185b4 blob - 658362effe9f7a546e607cdc600ad87c21b66f60 blob + 050a774d3a1387573a851619c802261f01af1e1f --- gotd/repo_read.c +++ gotd/repo_read.c @@ -286,8 +286,6 @@ list_refs(struct imsg *imsg) } client->id = ireq.client_id; client->fd = client_fd; - client->delta_cache_fd = -1; - client->pack_pipe = -1; imsg_init(&ibuf, client_fd); @@ -670,35 +668,6 @@ static const struct got_error * return err; } -static const struct got_error * -recv_disconnect(struct imsg *imsg) -{ - const struct got_error *err = NULL; - struct gotd_imsg_disconnect idisconnect; - size_t datalen; - int delta_cache_fd, pack_pipe; - struct repo_read_client *client = &repo_read_client; - - datalen = imsg->hdr.len - IMSG_HEADER_SIZE; - if (datalen != sizeof(idisconnect)) - return got_error(GOT_ERR_PRIVSEP_LEN); - memcpy(&idisconnect, imsg->data, sizeof(idisconnect)); - - log_debug("client disconnecting"); - - free_object_ids(&client->have_ids); - free_object_ids(&client->want_ids); - if (close(client->fd) == -1) - err = got_error_from_errno("close"); - delta_cache_fd = client->delta_cache_fd; - if (delta_cache_fd != -1 && close(delta_cache_fd) == -1 && err == NULL) - return got_error_from_errno("close"); - pack_pipe = client->pack_pipe; - if (pack_pipe != -1 && close(pack_pipe) == -1 && err == NULL) - return got_error_from_errno("close"); - return err; -} - static void repo_read_dispatch_session(int fd, short event, void *arg) { @@ -774,13 +743,6 @@ repo_read_dispatch_session(int fd, short event, void * log_warnx("%s: sending packfile: %s", repo_read.title, err->msg); break; - case GOTD_IMSG_DISCONNECT: - err = recv_disconnect(&imsg); - if (err) - log_warnx("%s: disconnect: %s", - repo_read.title, err->msg); - shut = 1; - break; default: log_debug("%s: unexpected imsg %d", repo_read.title, imsg.hdr.type); @@ -898,8 +860,13 @@ repo_read_main(const char *title, const char *repo_pat int *pack_fds, int *temp_fds) { const struct got_error *err = NULL; + struct repo_read_client *client = &repo_read_client; struct gotd_imsgev iev; + client->fd = -1; + client->delta_cache_fd = -1; + client->pack_pipe = -1; + repo_read.title = title; repo_read.pid = getpid(); repo_read.pack_fds = pack_fds; @@ -945,7 +912,19 @@ repo_read_shutdown(void) void repo_read_shutdown(void) { + struct repo_read_client *client = &repo_read_client; + log_debug("%s: shutting down", repo_read.title); + + free_object_ids(&client->have_ids); + free_object_ids(&client->want_ids); + if (client->fd != -1) + close(client->fd); + if (client->delta_cache_fd != -1) + close(client->delta_cache_fd); + if (client->pack_pipe != -1) + close(client->pack_pipe); + if (repo_read.repo) got_repo_close(repo_read.repo); got_repo_pack_fds_close(repo_read.pack_fds); blob - b425ba49c41d4c20c076b2c6c6a77b84366abf0d blob + ea3be8ec0af54941dbfa5189f510f3a126c52d2b --- gotd/repo_write.c +++ gotd/repo_write.c @@ -257,8 +257,6 @@ list_refs(struct imsg *imsg) } client->id = ireq.client_id; client->fd = client_fd; - client->pack_pipe = -1; - client->packidx_fd = -1; client->nref_updates = 0; client->nref_del = 0; client->nref_new = 0; @@ -1196,8 +1194,6 @@ recv_disconnect(struct imsg *imsg) const struct got_error *err = NULL; struct gotd_imsg_disconnect idisconnect; size_t datalen; - int pack_pipe = -1, idxfd = -1; - struct repo_write_client *client = &repo_write_client; datalen = imsg->hdr.len - IMSG_HEADER_SIZE; if (datalen != sizeof(idisconnect)) @@ -1206,22 +1202,6 @@ recv_disconnect(struct imsg *imsg) log_debug("client disconnecting"); - while (!STAILQ_EMPTY(&client->ref_updates)) { - struct gotd_ref_update *ref_update; - ref_update = STAILQ_FIRST(&client->ref_updates); - STAILQ_REMOVE_HEAD(&client->ref_updates, entry); - got_ref_close(ref_update->ref); - free(ref_update); - } - err = got_pack_close(&client->pack); - if (client->fd != -1 && close(client->fd) == -1) - err = got_error_from_errno("close"); - pack_pipe = client->pack_pipe; - if (pack_pipe != -1 && close(pack_pipe) == -1 && err == NULL) - err = got_error_from_errno("close"); - idxfd = client->packidx_fd; - if (idxfd != -1 && close(idxfd) == -1 && err == NULL) - err = got_error_from_errno("close"); return err; } @@ -1491,8 +1471,14 @@ repo_write_main(const char *title, const char *repo_pa int *pack_fds, int *temp_fds) { const struct got_error *err = NULL; + struct repo_write_client *client = &repo_write_client; struct gotd_imsgev iev; + client->fd = -1; + client->pack_pipe = -1; + client->packidx_fd = -1; + client->pack.fd = -1; + repo_write.title = title; repo_write.pid = getpid(); repo_write.pack_fds = pack_fds; @@ -1539,7 +1525,26 @@ repo_write_shutdown(void) void repo_write_shutdown(void) { + struct repo_write_client *client = &repo_write_client; + struct gotd_ref_update *ref_update; + log_debug("%s: shutting down", repo_write.title); + + while (!STAILQ_EMPTY(&client->ref_updates)) { + ref_update = STAILQ_FIRST(&client->ref_updates); + STAILQ_REMOVE_HEAD(&client->ref_updates, entry); + got_ref_close(ref_update->ref); + free(ref_update); + } + + got_pack_close(&client->pack); + if (client->fd != -1) + close(client->fd); + if (client->pack_pipe != -1) + close(client->pack_pipe); + if (client->packidx_fd != -1) + close(client->packidx_fd); + if (repo_write.repo) got_repo_close(repo_write.repo); got_repo_pack_fds_close(repo_write.pack_fds);