Fix gc segfault #47
Closed
Fix gc segfault #47
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Johannes Schindelin <[email protected]>
35 tasks
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]>
jamill
force-pushed
the
fix-gc-segfault
branch
from
b920785 to
8979693
Compare
|
/allow |
|
User undefined is now allowed to use GitGitGadget. |
|
/allow jamill |
|
User jamill already allowed to use GitGitGadget. |
|
@dscho I updated the PR description. I updated the test number to not conflict with other tests that have been added upstream. Is there anything else I should check before slash-submitting this? |
|
@jamill I think all that remains to do is: submit (see e.g. #31 (comment)). |
|
/submit |
|
Submitted as [email protected] |
|
This branch is now known as |
|
This patch series was integrated into pu via git@620b00e. |
|
This patch series was integrated into next via git@620b00e. |
|
This patch series was integrated into master via git@620b00e. |
|
Closed via 620b00e. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 added code to initialize
it, but at an incorrect spot: in
init_threaded_search(), while thecall to
oe_set_delta_size()(and hence topacking_data_lock()) canhappen in the call chain
check_object()<-get_object_details()<-prepare_pack()<-cmd_pack_objects(), which is long before theprepare_pack()function callsll_find_deltas()(which initializesthe 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(), whichnot only lives in libgit.a, but has to be called before the
packing_datastruct is used that contains that mutex.