Skip to content

Fix occasional segmentation fault in git gc #1853

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 28, 2018

Conversation

dscho
Copy link
Member

@dscho dscho commented Sep 27, 2018

It was reported in #1839 that git gc crashes under certain circumstances. The MCVE provided by @thomas-patzig and the initial analysis by @kgybels were invaluable to come up with this PR. Thank you!

Signed-off-by: Johannes Schindelin <[email protected]>
There is a problem in the way 9ac3f0e (pack-objects: fix
performance issues on packing large deltas, 2018-07-22) initializes that
mutex in the `packing_data` struct. The problem manifests in a
segmentation fault on Windows, when a mutex (AKA critical section) is
accessed without being initialized. (With pthreads, you apparently do
not really have to initialize them?)

This was reported in git-for-windows#1839.

Signed-off-by: Johannes Schindelin <[email protected]>
…spot

In 9ac3f0e (pack-objects: fix performance issues on packing large
deltas, 2018-07-22), a mutex was introduced that is used to guard the
call to set the delta size. This commit even added code to initialize
it, but at an incorrect spot: in `init_threaded_search()`, while the
call to `oe_set_delta_size()` (and hence to `packing_data_lock()`) can
happen in the call chain `check_object()` <- `get_object_details()` <-
`prepare_pack()` <- `cmd_pack_objects()`, which is long before the
`prepare_pack()` function calls `ll_find_deltas()` (which initializes
the threaded search).

Another tell-tale that the mutex was initialized in an incorrect spot is
that the function to initialize it lives in builtin/, while the code
that uses the mutex is defined in a libgit.a header file.

Let's use a more appropriate function: `prepare_packing_data()`, which
not only lives in libgit.a, but *has* to be called before the
`packing_data` struct is used that contains that mutex.

This fixes git-for-windows#1839.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho dscho requested review from jamill and kgybels September 27, 2018 20:52
@dscho dscho added this to the v2.19.0(2) milestone Sep 27, 2018
@dscho dscho merged commit 502e856 into git-for-windows:master Sep 28, 2018
@dscho dscho deleted the fix-gc-segfault branch September 28, 2018 08:59
dscho added a commit that referenced this pull request Oct 10, 2018
Fix occasional segmentation fault in `git gc`
dscho added a commit to dscho/git that referenced this pull request Oct 11, 2018
Fix occasional segmentation fault in `git gc`
dscho added a commit to dscho/git that referenced this pull request Nov 19, 2018
Fix occasional segmentation fault in `git gc`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants