Skip to content

Commit 46092eb

Browse files
committed
Merge branch 'jk/index-pack-threading-races' into maint
When receiving an invalid pack stream that records the same object twice, multiple threads got confused due to a race. * jk/index-pack-threading-races: index-pack: fix race condition with duplicate bases
2 parents 0605170 + ab791dd commit 46092eb

File tree

1 file changed

+31
-2
lines changed

1 file changed

+31
-2
lines changed

builtin/index-pack.c

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,10 @@ static pthread_mutex_t deepest_delta_mutex;
112112
#define deepest_delta_lock() lock_mutex(&deepest_delta_mutex)
113113
#define deepest_delta_unlock() unlock_mutex(&deepest_delta_mutex)
114114

115+
static pthread_mutex_t type_cas_mutex;
116+
#define type_cas_lock() lock_mutex(&type_cas_mutex)
117+
#define type_cas_unlock() unlock_mutex(&type_cas_mutex)
118+
115119
static pthread_key_t key;
116120

117121
static inline void lock_mutex(pthread_mutex_t *mutex)
@@ -135,6 +139,7 @@ static void init_thread(void)
135139
init_recursive_mutex(&read_mutex);
136140
pthread_mutex_init(&counter_mutex, NULL);
137141
pthread_mutex_init(&work_mutex, NULL);
142+
pthread_mutex_init(&type_cas_mutex, NULL);
138143
if (show_stat)
139144
pthread_mutex_init(&deepest_delta_mutex, NULL);
140145
pthread_key_create(&key, NULL);
@@ -157,6 +162,7 @@ static void cleanup_thread(void)
157162
pthread_mutex_destroy(&read_mutex);
158163
pthread_mutex_destroy(&counter_mutex);
159164
pthread_mutex_destroy(&work_mutex);
165+
pthread_mutex_destroy(&type_cas_mutex);
160166
if (show_stat)
161167
pthread_mutex_destroy(&deepest_delta_mutex);
162168
for (i = 0; i < nr_threads; i++)
@@ -862,7 +868,6 @@ static void resolve_delta(struct object_entry *delta_obj,
862868
{
863869
void *base_data, *delta_data;
864870

865-
delta_obj->real_type = base->obj->real_type;
866871
if (show_stat) {
867872
delta_obj->delta_depth = base->obj->delta_depth + 1;
868873
deepest_delta_lock();
@@ -888,6 +893,26 @@ static void resolve_delta(struct object_entry *delta_obj,
888893
counter_unlock();
889894
}
890895

896+
/*
897+
* Standard boolean compare-and-swap: atomically check whether "*type" is
898+
* "want"; if so, swap in "set" and return true. Otherwise, leave it untouched
899+
* and return false.
900+
*/
901+
static int compare_and_swap_type(enum object_type *type,
902+
enum object_type want,
903+
enum object_type set)
904+
{
905+
enum object_type old;
906+
907+
type_cas_lock();
908+
old = *type;
909+
if (old == want)
910+
*type = set;
911+
type_cas_unlock();
912+
913+
return old == want;
914+
}
915+
891916
static struct base_data *find_unresolved_deltas_1(struct base_data *base,
892917
struct base_data *prev_base)
893918
{
@@ -915,7 +940,10 @@ static struct base_data *find_unresolved_deltas_1(struct base_data *base,
915940
struct object_entry *child = objects + deltas[base->ref_first].obj_no;
916941
struct base_data *result = alloc_base_data();
917942

918-
assert(child->real_type == OBJ_REF_DELTA);
943+
if (!compare_and_swap_type(&child->real_type, OBJ_REF_DELTA,
944+
base->obj->real_type))
945+
die("BUG: child->real_type != OBJ_REF_DELTA");
946+
919947
resolve_delta(child, base, result);
920948
if (base->ref_first == base->ref_last && base->ofs_last == -1)
921949
free_base_data(base);
@@ -929,6 +957,7 @@ static struct base_data *find_unresolved_deltas_1(struct base_data *base,
929957
struct base_data *result = alloc_base_data();
930958

931959
assert(child->real_type == OBJ_OFS_DELTA);
960+
child->real_type = base->obj->real_type;
932961
resolve_delta(child, base, result);
933962
if (base->ofs_first == base->ofs_last)
934963
free_base_data(base);

0 commit comments

Comments
 (0)