Re: Segfault in master due to 4fbcca4eff

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Fri, Sep 21 2018, Derrick Stolee wrote:

> On 9/21/2018 10:30 AM, Ævar Arnfjörð Bjarmason wrote:
>> On Fri, Sep 21 2018, Junio C Hamano wrote:
>>
>>> * ds/reachable (2018-08-28) 19 commits
>>>    (merged to 'next' on 2018-08-28 at b1634b371d)
>>>   + commit-reach: correct accidental #include of C file
>>>    (merged to 'next' on 2018-08-22 at 17f3275afb)
>>>   + commit-reach: use can_all_from_reach
>>>   + commit-reach: make can_all_from_reach... linear
>>>   + commit-reach: replace ref_newer logic
>>>   + test-reach: test commit_contains
>>>   + test-reach: test can_all_from_reach_with_flags
>>>   + test-reach: test reduce_heads
>>>   + test-reach: test get_merge_bases_many
>>>   + test-reach: test is_descendant_of
>>>   + test-reach: test in_merge_bases
>>>   + test-reach: create new test tool for ref_newer
>>>   + commit-reach: move can_all_from_reach_with_flags
>>>   + upload-pack: generalize commit date cutoff
>>>   + upload-pack: refactor ok_to_give_up()
>>>   + upload-pack: make reachable() more generic
>>>   + commit-reach: move commit_contains from ref-filter
>>>   + commit-reach: move ref_newer from remote.c
>>>   + commit.h: remove method declarations
>>>   + commit-reach: move walk methods from commit.c
>>>
>>>   The code for computing history reachability has been shuffled,
>>>   obtained a bunch of new tests to cover them, and then being
>>>   improved.
>> There's a segfault now in master when fetching because of 4fbcca4eff
>> ("commit-reach: make can_all_from_reach... linear", 2018-07-20). I have
>> not had time to debug this, or found an easy isolated test case, but
>> this script will reliably make it segfault for me:
>>
>>      #!/bin/sh
>>
>>      git_dot_git=/home/avar/g/git
>>
>>      old=v0.99
>>      new=v0.99.1
>>
>>      rm -rf /tmp/srv
>>      rm -rf /tmp/cln
>>      git init --bare /tmp/srv
>>      git init --bare /tmp/cln
>>      $git_dot_git/git --exec-path=$git_dot_git push file:///tmp/srv $old:refs/whatever/ref
>>      $git_dot_git/git --exec-path=$git_dot_git -C /tmp/cln fetch file:///tmp/srv 'refs/*:refs/*'
>>      $git_dot_git/git --exec-path=$git_dot_git push file:///tmp/srv $new:refs/whatever/ref
>>      if GIT_TRACE=1 $git_dot_git/git --exec-path=$git_dot_git -C /tmp/cln fetch file:///tmp/srv 'refs/*:refs/*'
>>      then
>>          exit 0
>>      else
>>          exit 1
>>      fi
>>
>> I.e. you need to push the v0.99 tag to its own repo, fetch that from
>> another one, then push v0.99.1, fetch everything, and you'll get a
>> segfault:
>>
>>      remote: Resolving deltas: 100% (187/187), completed with 48 local objects.
>>      To file:///tmp/srv
>>         d6602ec519..f25a265a34  v0.99.1 -> refs/whatever/ref
>>      14:26:44.505787 git.c:415               trace: built-in: git fetch file:///tmp/srv 'refs/*:refs/*'
>>      14:26:44.506708 run-command.c:637       trace: run_command: unset GIT_DIR GIT_IMPLICIT_WORK_TREE GIT_PREFIX; 'git-upload-pack '\''/tmp/srv'\'''
>>      14:26:44.508831 git.c:415               trace: built-in: git upload-pack /tmp/srv
>>      14:26:44.509953 run-command.c:637       trace: run_command: git rev-list --objects --stdin --not --all --quiet
>>      Segmentation fault
>>      fatal: The remote end hung up unexpectedly
>>
>> Same with refs/tags/ref b.t.w., not just refs/whatever/ref, I just was
>> initially testing this for some follow-up work on my series for checking
>> how fetching to various non-heads/tags namespaces works.
>
> This error was reported by Peff [1] and fixed in [2], but as stated
> [3] I was waiting for more review before sending a v3. I'll send that
> v3 shortly, responding to the feedback so far.
>
> -Stolee
>
> [1]
> https://public-inbox.org/git/0bf9103c-9377-506b-7ad7-e5273d8e94fc@gmail.com/T/#u
>
> [2] https://public-inbox.org/git/pull.39.git.gitgitgadget@gmail.com/
>
> [3]
> https://public-inbox.org/git/8d6061de-1654-577c-40c6-211dbd03aa36@gmail.com/

Thanks and sorry for the duplicate report. I can confirm that applying
the v2 of that fixes the segfault for the test case I posted.