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

[PATCH v2 0/7] remote-curl: fix deadlocks when remote server disconnects

From
Denton Liu <liu.denton@gmail.com>
Date
May 18, 2020, 15:47 UTC
Message-ID
<cover.1589816718.git.liu.denton@gmail.com>
In-Reply-To
<cover.1589393036.git.liu.denton@gmail.com>
The following command hangs forever:
	$ git -c protocol.version=2 clone https://github.com/git/git.git --shallow-since=20151012
	Cloning into 'git'...

This occurs because the --shallow-since arg is incorrect and the server dies early. However, remote-curl does not realise that the server errored out and just faithfully forwards the packets to fetch-pack before waiting on more input from fetch-pack. Meanwhile, fetch-pack keeps reading as it still expects more input. As a result, the processes deadlock. Original analysis by Peff: https://lore.kernel.org/git/20200328154936.GA1217052@coredump.intra.peff.net/

Changes since v1:
* Remove fallthrough in switch in favour of just extracting the common
  call out of the switch in patch 3
* Add more detail in function comment and use `const char linelen[4]` in
  patch 4
* Implement most of Peff's suggestions[0] in patch 5
* Only operate on stateless_connect() in patch 5
* Add tests in patch 5
* Drop "remote-curl: ensure last packet is a flush" in favour of
  "stateless-connect: send response end packet"
[0]: https://lore.kernel.org/git/20200515213844.GD115445@coredump.intra.peff.net/
Denton Liu (7):
  remote-curl: fix typo
  remote-curl: remove label indentation
  transport: extract common fetch_pack() call
  pkt-line: extern packet_length()
  remote-curl: error on incomplete packet
  pkt-line: PACKET_READ_RESPONSE_END
  stateless-connect: send response end packet
 Documentation/gitremote-helpers.txt           |  4 +-
 Documentation/technical/protocol-v2.txt       |  2 +
 builtin/fetch-pack.c                          |  2 +-
 connect.c                                     | 12 +++-
 fetch-pack.c                                  | 12 ++++
 pkt-line.c                                    | 13 +++-
 pkt-line.h                                    | 11 +++
 remote-curl.c                                 | 72 +++++++++++++++++--
 remote.h                                      |  3 +-
 serve.c                                       |  2 +
 t/helper/test-pkt-line.c                      |  4 ++
 t/lib-httpd.sh                                |  2 +
 t/lib-httpd/apache.conf                       |  8 +++
 .../incomplete-body-upload-pack-v2-http.sh    |  3 +
 .../incomplete-length-upload-pack-v2-http.sh  |  3 +
 t/t5702-protocol-v2.sh                        | 47 ++++++++++++
 transport.c                                   | 16 ++---
 17 files changed, 197 insertions(+), 19 deletions(-)
 create mode 100644 t/lib-httpd/incomplete-body-upload-pack-v2-http.sh
 create mode 100644 t/lib-httpd/incomplete-length-upload-pack-v2-http.sh
Range-diff against v1:
1:  b390875f87 = 1:  b390875f87 remote-curl: fix typo
2:  a2b28c0b28 = 2:  a2b28c0b28 remote-curl: remove label indentation
3:  c89c184100 < -:  ---------- transport: combine common cases with a fallthrough
-:  ---------- > 3:  3a42575bd5 transport: extract common fetch_pack() call
4:  891a39c853 ! 4:  c2b9d033bb pkt-line: extern packet_length()
    @@ Commit message
         need to access the length header. In order to simplify this, extern
         packet_length() so that the logic can be reused.
     
    +    Change the function parameter from a `const char *` to
    +    `const char linelen[4]`. Even though these two types behave identically
    +    as function parameters, use the array notation to semantically indicate
    +    exactly what this function is expecting as an argument.
    +
      ## pkt-line.c ##
     @@ pkt-line.c: static int get_packet_data(int fd, char **src_buf, size_t *src_size,
      	return ret;
      }
      
     -static int packet_length(const char *linelen)
    -+int packet_length(const char *linelen)
    ++int packet_length(const char linelen[4])
      {
      	int val = hex2chr(linelen);
      	return (val < 0) ? val : (val << 8) | hex2chr(linelen + 2);
    @@ pkt-line.h: int write_packetized_from_buf(const char *src_in, size_t len, int fd
      		*buffer, unsigned size, int options);
      
     +/*
    -+ * Reads a packetized line and returns the length header of the packet.
    ++ * Convert a four hex digit packet line length header into its numeric
    ++ * representation. linelen should not be null-terminated.
    ++ *
    ++ * If linelen contains non-hex characters, return -1. Otherwise, return the
    ++ * numeric value of the length header.
     + */
    -+int packet_length(const char *linelen);
    ++int packet_length(const char linelen[4]);
     +
      /*
       * Read a packetized line into a buffer like the 'packet_read()' function but
5:  3ed7cf87aa < -:  ---------- remote-curl: error on incomplete packet
6:  7a689da2bb < -:  ---------- remote-curl: ensure last packet is a flush
-:  ---------- > 5:  52ce5fdffd remote-curl: error on incomplete packet
-:  ---------- > 6:  744b078324 pkt-line: PACKET_READ_RESPONSE_END
-:  ---------- > 7:  4b079bcd83 stateless-connect: send response end packet
-- 
2.26.2.706.g87896c9627
Previous: Jeff KingNext: Denton Liu
Message 22 of 62 in “remote-curl: partial fix for a deadlock with stateless rpc”
  1. 0/6 remote-curl: partial fix for a deadlock with stateless rpcDenton Liu, May 13, 2020
  2. 1/6 remote-curl: fix typoDenton Liu, May 13, 2020
  3. 2/6 remote-curl: remove label indentationDenton Liu, May 13, 2020
  4. 3/6 transport: combine common cases with a fallthroughDenton Liu, May 13, 2020
  5. Eric SunshineMay 13, 2020
  6. Denton LiuMay 18, 2020
  7. Eric SunshineMay 18, 2020
  8. 4/6 pkt-line: extern packet_length()Denton Liu, May 13, 2020
  9. Eric SunshineMay 13, 2020
  10. Jeff KingMay 15, 2020
  11. Jeff KingMay 15, 2020
  12. 5/6 remote-curl: error on incomplete packetDenton Liu, May 13, 2020
  13. Jeff KingMay 15, 2020
  14. Denton LiuMay 18, 2020
  15. Jeff KingMay 18, 2020
  16. 6/6 remote-curl: ensure last packet is a flushDenton Liu, May 13, 2020
  17. Denton LiuMay 15, 2020
  18. Jeff KingMay 15, 2020
  19. Junio C HamanoMay 18, 2020
  20. Jeff KingMay 18, 2020
  21. Jeff KingMay 18, 2020
  22. 0/7 remote-curl: fix deadlocks when remote server disconnectsDenton Liu, May 18, 2020
  23. 1/7 remote-curl: fix typoDenton Liu, May 18, 2020
  24. 2/7 remote-curl: remove label indentationDenton Liu, May 18, 2020
  25. Junio C HamanoMay 18, 2020
  26. 4/7 pkt-line: extern packet_length()Denton Liu, May 18, 2020
  27. Jeff KingMay 18, 2020
  28. Eric SunshineMay 18, 2020
  29. Jeff KingMay 18, 2020
  30. Junio C HamanoMay 18, 2020
  31. 3/7 transport: extract common fetch_pack() callDenton Liu, May 18, 2020
  32. Junio C HamanoMay 18, 2020
  33. 6/7 pkt-line: PACKET_READ_RESPONSE_ENDDenton Liu, May 18, 2020
  34. 7/7 stateless-connect: send response end packetDenton Liu, May 18, 2020
  35. Jeff KingMay 18, 2020
  36. Denton LiuMay 18, 2020
  37. Jeff KingMay 18, 2020
  38. 5/7 remote-curl: error on incomplete packetDenton Liu, May 18, 2020
  39. Jeff KingMay 18, 2020
  40. Denton LiuMay 18, 2020
  41. Jeff KingMay 18, 2020
  42. Denton LiuMay 18, 2020
  43. Jeff KingMay 18, 2020
  44. Junio C HamanoMay 18, 2020
  45. Jeff KingMay 19, 2020
  46. Junio C HamanoMay 18, 2020
  47. 0/7 remote-curl: fix deadlocks when remote server disconnectsDenton Liu, May 19, 2020
  48. 1/7 remote-curl: fix typoDenton Liu, May 19, 2020
  49. 2/7 remote-curl: remove label indentationDenton Liu, May 19, 2020
  50. 3/7 transport: extract common fetch_pack() callDenton Liu, May 19, 2020
  51. 4/7 pkt-line: extern packet_length()Denton Liu, May 19, 2020
  52. Eric SunshineMay 19, 2020
  53. 6/7 pkt-line: define PACKET_READ_RESPONSE_ENDDenton Liu, May 19, 2020
  54. 5/7 remote-curl: error on incomplete packetDenton Liu, May 19, 2020
  55. 7/7 stateless-connect: send response end packetDenton Liu, May 19, 2020
  56. Jeff KingMay 19, 2020
  57. Denton LiuMay 19, 2020
  58. 8/7 fixup! pkt-line: extern packet_length()Denton Liu, May 19, 2020
  59. 9/9 fixup! remote-curl: error on incomplete packetDenton Liu, May 22, 2020
  60. Jeff KingMay 22, 2020
  61. Denton LiuMay 22, 2020
  62. Jeff KingMay 22, 2020

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.