Re: [PATCH 3/3] Add sideband status report to git-archive protocol
- From
- Franck Bui-Huu <vagabon.xyz@gmail.com>
- Date
- Sep 11, 2006, 10:34 UTC
- Message-ID
- <45053BA2.6050502@innova-card.com>
- In-Reply-To
- <7v1wqkt2v4.fsf_-_@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
Show 6 quoted lines
> Using the refactored sideband code from existing upload-pack protocol, > this lets the error condition and status output sent from the remote > process to be shown locally. > > Signed-off-by: Junio C Hamano <junkio@cox.net> > ---
[snip]
Show 39 quoted lines
> -}
> + while (1) {
> + struct pollfd pfd[2];
> + char buf[16384];
> + ssize_t sz;
> + pid_t pid;
> + int status;
> +
> + pfd[0].fd = fd1[0];
> + pfd[0].events = POLLIN;
> + pfd[1].fd = fd2[0];
> + pfd[1].events = POLLIN;
> + if (poll(pfd, 2, -1) < 0) {
> + if (errno != EINTR) {
> + error("poll failed resuming: %s",
> + strerror(errno));
> + sleep(1);
> + }
> + continue;
> + }
> + if (pfd[0].revents & (POLLIN|POLLHUP)) {
> + /* Data stream ready */
> + sz = read(pfd[0].fd, buf, sizeof(buf));
> + send_sideband(1, 1, buf, sz, DEFAULT_PACKET_MAX);
> + }
> + if (pfd[1].revents & (POLLIN|POLLHUP)) {
> + /* Status stream ready */
> + sz = read(pfd[1].fd, buf, sizeof(buf));
> + send_sideband(1, 2, buf, sz, DEFAULT_PACKET_MAX);
> + }
>
> + if (((pfd[0].revents | pfd[1].revents) & POLLHUP) == 0)
> + continue;
> + /* did it die? */
> + pid = waitpid(writer, &status, WNOHANG);
> + if (!pid) {
> + fprintf(stderr, "Hmph, HUP?\n");
> + continue;
> + }I get a lot of "Hmph, HUP?" messages when testing "git-archive --remote" command. One guess: this can be due to the fact that when the writer process exits, it first closes its fd but do not send a SIGCHLD signal right after to its parent.
Therefore poll() can return POLLHUP flag to the parent process but waitpid still returns 0 because the writer process has still not sent SIGCHLD signal to its parent.
How about this patch ? If poll() doesn't only return the single POLLIN flag, then either the pipe has been closed because the write process died or something wrong happened. In all these cases we can wait for the writer process to exit then die.
-- >8 --
diff --git a/builtin-upload-archive.c b/builtin-upload-archive.c index 42cb9f8..2ebe9a0 100644 --- a/builtin-upload-archive.c +++ b/builtin-upload-archive.c @@ -114,7 +114,6 @@ int cmd_upload_archive(int argc, const c struct pollfd pfd[2]; char buf[16384]; ssize_t sz; - pid_t pid; int status; pfd[0].fd = fd1[0]; @@ -140,13 +139,11 @@ int cmd_upload_archive(int argc, const c send_sideband(1, 2, buf, sz, LARGE_PACKET_MAX); } - if (((pfd[0].revents | pfd[1].revents) & POLLHUP) == 0) - continue; - /* did it die? */ - pid = waitpid(writer, &status, WNOHANG); - if (!pid) { - fprintf(stderr, "Hmph, HUP?\n"); + if ((pfd[0].revents | pfd[1].revents) == POLLIN) continue; + + if (waitpid(writer, &status, 0) < 0) { + die("waitpid failed: %s", strerror(errno)); } if (!WIFEXITED(status) || WEXITSTATUS(status) > 0) send_sideband(1, 3, deadchild, strlen(deadchild),