git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/2] send-pack: avoid deadlock when pack-object dies early

From
Johannes Sixt <j6t@kdbg.org>
Date
Apr 24, 2011, 20:49 UTC
Message-ID
<4DB48CCD.40304@kdbg.org>
In-Reply-To
<4DB48B2C.2090904@kdbg.org>

When pack-objects dies prematurely (for example, because there is some repo corruption), we are careful to clean up the sideband demuxer (if it is being used) with finish_async. For an async implementation which forks (i.e., not Windows), that means we will waitpid() for the async process.

Meanwhile, the async sideband demuxer will continue trying to stream data from the remote repo until it gets EOF. Depending on what data pack-objects actually sent, the remote repo may not actually send anything, in particular, after the initial rev-exchange it is waiting for the pack data to arrive.

The send-pack parent process closes the writable end of the channel so that after the death of the pack-objects process all writable ends should have been closed and the remote repo should see EOF. This does not happen, however, because when the sideband demuxer was forked earlier, it also inherited a writable end; it remains open and keeps the remote repo from seeing EOF. This leads to a deadlock cycle in which send-pack waits on the demuxer, the demuxer waits on the remote receive-pack, and the remote receive-pack waits on send-pack to send the pack data.

To break this, we close the writable end in the demuxer, but only when it runs in a forked async process.

Analyzed-by: Jeff King <peff@peff.net>
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
This is the replacement of the series that Jeff proposed earlier. It
should also be merged into f6b60983 (Enable threaded async procedures
whenever pthreads is available). To keep bisectability, the merge commit
should replace '#ifndef ASYNC_AS_THREAD' by '#ifdef NO_PTHREADS'.
-- Hannes
 builtin-send-pack.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 089058b..b371c79 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -376,6 +376,9 @@ static void print_helper_status(struct ref *ref)
 static int sideband_demux(int in, int out, void *data)
 {
 	int *fd = data;
+#ifndef ASYNC_AS_THREAD
+	close(fd[1]);
+#endif
 	int ret = recv_sideband("send-pack", fd[0], out);
 	close(out);
 	return ret;
-- 
1.7.5.rc1.97.ge0653
Previous: Johannes SixtNext: Jeff King
Message 19 of 48 in “fix hang in git push when pack-objects fails”
  1. 0/4 fix hang in git push when pack-objects failsJeff King, Mar 31, 2011
  2. 1/4 teach wait_or_whine a "quiet" modeJeff King, Mar 31, 2011
  3. Johannes SixtMar 31, 2011
  4. Jeff KingApr 1, 2011
  5. 2/4 finish_async: be quiet when waiting for async processJeff King, Mar 31, 2011
  6. 3/4 run-command: allow aborting async code prematurelyJeff King, Mar 31, 2011
  7. Erik Faye-LundApr 1, 2011
  8. Jeff KingApr 1, 2011
  9. 4/4 send-pack: abort sideband demuxer on pack-objects errorJeff King, Mar 31, 2011
  10. Johannes SixtApr 13, 2011
  11. Jeff KingApr 14, 2011
  12. Johannes SixtApr 14, 2011
  13. Jeff KingApr 14, 2011
  14. Johannes SixtApr 14, 2011
  15. Jeff KingApr 14, 2011
  16. Johannes SixtApr 14, 2011
  17. Junio C HamanoApr 14, 2011
  18. 1/2 send-pack --stateless-rpc: properly close the outgoing channelJohannes Sixt, Apr 24, 2011
  19. 2/2 send-pack: avoid deadlock when pack-object dies earlyJohannes Sixt, Apr 24, 2011
  20. Jeff KingApr 25, 2011
  21. Johannes SixtApr 25, 2011
  22. Junio C HamanoApr 25, 2011
  23. send-pack: avoid deadlock when pack-object dies earlyJohannes Sixt, Apr 25, 2011
  24. Jeff KingApr 26, 2011
  25. Jeff KingApr 25, 2011
  26. 5/4 run-command: implement abort_async for pthreadsJeff King, Mar 31, 2011
  27. Erik Faye-LundApr 1, 2011
  28. Erik Faye-LundApr 1, 2011
  29. Johannes SixtApr 1, 2011
  30. Jeff KingApr 1, 2011
  31. Erik Faye-LundApr 1, 2011
  32. Erik Faye-LundApr 1, 2011
  33. Johannes SixtApr 1, 2011
  34. Erik Faye-LundApr 1, 2011
  35. Jeff KingApr 1, 2011
  36. Erik Faye-LundApr 1, 2011
  37. Jeff KingApr 1, 2011
  38. Jeff KingApr 1, 2011
  39. Erik Faye-LundApr 1, 2011
  40. Johannes SixtApr 1, 2011
  41. Erik Faye-LundApr 1, 2011
  42. Johannes SixtApr 1, 2011
  43. Erik Faye-LundApr 1, 2011
  44. Jeff KingApr 1, 2011
  45. Erik Faye-LundApr 2, 2011
  46. Jeff KingApr 1, 2011
  47. Johannes SixtMar 31, 2011
  48. Jeff KingApr 1, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.