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

Re: [PATCH v9 00/19] index-helper/watchman

From
Dennis Kaarsemaker <dennis@kaarsemaker.net>
Date
May 10, 2016, 17:44 UTC
Message-ID
<1462902241.11954.10.camel@kaarsemaker.net>
In-Reply-To
<1462901304.11954.5.camel@kaarsemaker.net>
On di, 2016-05-10 at 19:28 +0200, Dennis Kaarsemaker wrote:
Show 126 quoted lines
> On ma, 2016-05-09 at 15:32 -0700, Junio C Hamano wrote:
> > 
> > Junio C Hamano <gitster@pobox.com> writes:
> > 
> > > 
> > > 
> > > David Turner <dturner@twopensource.com> writes:
> > > 
> > > > 
> > > > 
> > > > On Mon, 2016-05-09 at 14:40 -0700, Junio C Hamano wrote:
> > > > > 
> > > > > 
> > > > > Hmmm, I seem to be getting
> > > > > 
> > > > >     $ cat t/trash*7900*/err
> > > > >     fatal: Already running
> > > > > 
> > > > > after running t7900 and it fails at #5, after applying
> > > > > "index-helper: optionally automatically run"
> > The symptom looks pretty similar to $gmane/293461 reported earlier.
> > Here is how "t7900-index-helper.sh -i -v -x -d" ends.
> > 
> > 
> > expecting success:
> >         test_when_finished "git index-helper --kill" &&
> >         rm -f .git/index-helper.sock &&
> >         git status &&
> >         test_path_is_missing .git/index-helper.sock &&
> >         test_config indexhelper.autorun true &&
> >         git status &&
> >         test -S .git/index-helper.sock &&
> >         git status 2>err &&
> >         test -S .git/index-helper.sock &&
> >         test_must_be_empty err &&
> >         git index-helper --kill &&
> >         test_config indexhelper.autorun false &&
> >         git status &&
> >         test_path_is_missing .git/index-helper.sock
> > 
> > + test_when_finished git index-helper --kill
> > + test 0 = 0
> > + test_cleanup={ git index-helper --kill
> >                 } && (exit "$eval_ret"); eval_ret=$?; :
> > + rm -f .git/index-helper.sock
> > + git status
> > On branch master
> > Untracked files:
> >   (use "git add <file>..." to include in what will be committed)
> > 
> >         err
> > 
> > nothing added to commit but untracked files present (use "git add"
> > to
> > track)
> > + test_path_is_missing .git/index-helper.sock
> > + test -e .git/index-helper.sock
> > + test_config indexhelper.autorun true
> > + config_dir=
> > + test indexhelper.autorun = -C
> > + test_when_finished test_unconfig  'indexhelper.autorun'
> > + test 0 = 0
> > + test_cleanup={ test_unconfig  'indexhelper.autorun'
> >                 } && (exit "$eval_ret"); eval_ret=$?; { git index-
> > helper --kill
> >                 } && (exit "$eval_ret"); eval_ret=$?; :
> > + git config indexhelper.autorun true
> > + git status
> > error: last command exited with $?=141
> > not ok 5 - index-helper autorun works
> > #
> > #               test_when_finished "git index-helper --kill" &&
> > #               rm -f .git/index-helper.sock &&
> > #               git status &&
> > #               test_path_is_missing .git/index-helper.sock &&
> > #               test_config indexhelper.autorun true &&
> > #               git status &&
> > #               test -S .git/index-helper.sock &&
> > #               git status 2>err &&
> > #               test -S .git/index-helper.sock &&
> > #               test_must_be_empty err &&
> > #               git index-helper --kill &&
> > #               test_config indexhelper.autorun false &&
> > #               git status &&
> > #               test_path_is_missing .git/index-helper.sock
> > #
> Here are the relevant bits of a strace, pid 22200 is the second git
> status, 222197 is the index helper. 22122 is the test script
> 
> 22200 socket(PF_LOCAL, SOCK_STREAM, 0)  = 7
> 22200 connect(7, {sa_family=AF_LOCAL, sun_path=".git/index-
> helper.sock"}, 110 <unfinished ...>
> 22197 <... poll resumed> )              = 1
> 22197 accept(7, 0, NULL)                = 8
> 22197 fcntl(8, F_GETFL)                 = 0x2 (flags O_RDWR)
> 22197 fcntl(8, F_SETFL, O_RDWR)         = 0
> 22197 read(8,  <unfinished ...>
> 22200 <... connect resumed> )           = 0
> 22200 rt_sigaction(SIGPIPE, {SIG_IGN, [PIPE], SA_RESTORER|SA_RESTART,
> 0x7fcc463fdd40}, {SIG_DFL, [PIPE], SA_RESTORER|SA_RESTART,
> 0x7fcc463fdd40}, 8) = 0
> 22200 write(7, "000fpoke 22200 ", 15 <unfinished ...>
> 22197 <... read resumed> 0x7ffc4e4b9b20, 4) = 4
> 22197 read(8, 0x7ffc4e4b9c70, 11)       = 11
> 22197 write(8, 0x18b08b0, 6)            = 6
> 22197 close(8)                          = 0
> 22197 poll([?] 0x7ffc4e4b9b80, 1, 600000 <unfinished ...>
> 22200 <... write resumed> )             = 15
> 22200 write(7, "0000", 4)               = -1 EPIPE (Broken pipe)
> 22200 --- SIGPIPE {si_signo=SIGPIPE, si_code=SI_USER, si_pid=22200,
> si_uid=1000} ---
> 22200 rt_sigaction(SIGPIPE, {SIG_DFL, [PIPE], SA_RESTORER|SA_RESTART,
> 0x7fcc463fdd40}, {SIG_IGN, [PIPE], SA_RESTORER|SA_RESTART,
> 0x7fcc463fdd40}, 8) = 0
> 22200 tgkill(22200, 22200, SIGPIPE)     = 0   
> 22200 --- SIGPIPE {si_signo=SIGPIPE, si_code=SI_TKILL, si_pid=22200,
> si_uid=1000} ---
> 22200 +++ killed by SIGPIPE +++
> 22122 <... wait4 resumed> [{WIFSIGNALED(s) && WTERMSIG(s) ==
> SIGPIPE}], 0, NULL) = 22200 
> 22122 --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_KILLED,
> si_pid=22200, si_status=SIGPIPE, si_utime=0, si_stime=0} ---
> 
> Looks like the index helper closes the socket, but git status still
> wants to write to it. The index-helper also doesn't seem to read all
> data from the socket.

This dodgy hack to make the index helper read that empty pkt-line before closing the socket seems to work around the issue enough to make the test pass

diff --git a/index-helper.c b/index-helper.c
index 9743481..fa072de 100644
--- a/index-helper.c
+++ b/index-helper.c
@@ -385,6 +385,8 @@ static void loop(int fd, int idle_in_seconds)
                         * close up.
                         */
                }
+               bytes_read = packet_read(client_fd, NULL, NULL, buf,
+                                        sizeof(buf), flags);
                close(client_fd);
        }
 
And this even dodgier hack breaks the race on a physical machine, so
the test reliably fails everywher:

diff --git a/read-cache.c b/read-cache.c
index e8593bc..848a1fe 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1836,6 +1836,7 @@ static int poke_and_wait_for_reply(int fd)
 
        strbuf_addf(&buf, "poke %d %s", getpid(), requested_capabilities);
        packet_write(fd, buf.buf, buf.len);
+       sleep(5);
        packet_flush(fd);
 
        /* Now wait for a reply */

But at this point I'll have to claim ignorance on whether reading the
empty pkt-line is a fix or just hiding the problem.

-- 
Dennis Kaarsemaker
www.kaarsemaker.net
Previous: Dennis KaarsemakerNext: Dennis Kaarsemaker
Message 35 of 39 in “index-helper/watchman”
  1. 00/19 index-helper/watchmanDavid Turner, May 9, 2016
  2. 01/19 read-cache.c: fix constness of verify_hdr()David Turner, May 9, 2016
  3. 02/19 read-cache: allow to keep mmap'd memory after readingDavid Turner, May 9, 2016
  4. 03/19 index-helper: new daemon for caching index and related stuffDavid Turner, May 9, 2016
  5. SZEDER GáborMay 10, 2016
  6. Duy NguyenMay 10, 2016
  7. SZEDER GáborMay 10, 2016
  8. Duy NguyenMay 10, 2016
  9. David TurnerMay 10, 2016
  10. 04/19 index-helper: add --strictDavid Turner, May 9, 2016
  11. 05/19 index-helper: log warningsDavid Turner, May 9, 2016
  12. 06/19 daemonize(): set a flag before exiting the main processDavid Turner, May 9, 2016
  13. 07/19 index-helper: add --detachDavid Turner, May 9, 2016
  14. 08/19 read-cache: add watchman 'WAMA' extensionDavid Turner, May 9, 2016
  15. 09/19 watchman: support watchman to reduce index refresh costDavid Turner, May 9, 2016
  16. 10/19 index-helper: use watchman to avoid refreshing index with lstat()David Turner, May 9, 2016
  17. 11/19 update-index: enable/disable watchman supportDavid Turner, May 9, 2016
  18. 12/19 unpack-trees: preserve index extensionsDavid Turner, May 9, 2016
  19. 13/19 watchman: add a config option to enable the extensionDavid Turner, May 9, 2016
  20. 14/19 index-helper: kill modeDavid Turner, May 9, 2016
  21. 15/19 index-helper: don't run if already runningDavid Turner, May 9, 2016
  22. 16/19 index-helper: autorun modeDavid Turner, May 9, 2016
  23. 17/19 index-helper: optionally automatically runDavid Turner, May 9, 2016
  24. 18/19 trace: measure where the time is spent in the index-heavy operationsDavid Turner, May 9, 2016
  25. 19/19 untracked-cache: config optionDavid Turner, May 9, 2016
  26. Junio C HamanoMay 9, 2016
  27. David TurnerMay 9, 2016
  28. Junio C HamanoMay 9, 2016
  29. Junio C HamanoMay 9, 2016
  30. David TurnerMay 9, 2016
  31. Ramsay JonesMay 9, 2016
  32. Duy NguyenMay 10, 2016
  33. Duy NguyenMay 10, 2016
  34. Dennis KaarsemakerMay 10, 2016
  35. Dennis KaarsemakerMay 10, 2016
  36. Dennis KaarsemakerMay 10, 2016
  37. Ramsay JonesMay 10, 2016
  38. Junio C HamanoMay 10, 2016
  39. Ramsay JonesMay 10, 2016

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.