Skip to content

Commit 0c2ad00

Browse files
Martin Ågrengitster
authored andcommitted
pack-objects: take lock before accessing remaining
When checking the conditional of "while (me->remaining)", we did not hold the lock. Calling find_deltas would still be safe, since it checks "remaining" (after taking the lock) and is able to handle all values. In fact, this could (currently) not trigger any bug: a bug could happen if `remaining` transitioning from zero to non-zero races with the evaluation of the while-condition, but these are always separated by the data_ready-mechanism. Make sure we have the lock when we read `remaining`. This does mean we release it just so that find_deltas can take it immediately again. We could tweak the contract so that the lock should be taken before calling find_deltas, but let's defer that until someone can actually show that "unlock+lock" has a measurable negative impact. Signed-off-by: Martin Ågren <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 5c94c93 commit 0c2ad00

File tree

1 file changed

+6
-0
lines changed

1 file changed

+6
-0
lines changed

builtin/pack-objects.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2171,7 +2171,10 @@ static void *threaded_find_deltas(void *arg)
21712171
{
21722172
struct thread_params *me = arg;
21732173

2174+
progress_lock();
21742175
while (me->remaining) {
2176+
progress_unlock();
2177+
21752178
find_deltas(me->list, &me->remaining,
21762179
me->window, me->depth, me->processed);
21772180

@@ -2193,7 +2196,10 @@ static void *threaded_find_deltas(void *arg)
21932196
pthread_cond_wait(&me->cond, &me->mutex);
21942197
me->data_ready = 0;
21952198
pthread_mutex_unlock(&me->mutex);
2199+
2200+
progress_lock();
21962201
}
2202+
progress_unlock();
21972203
/* leave ->working 1 so that this doesn't get more work assigned */
21982204
return NULL;
21992205
}

0 commit comments

Comments
 (0)