Skip to content

Commit f3c9ca0

Browse files
authored
Merge pull request #833 from fjtrujy/fix/libcglue-retarget-lock-recursive-race
fix: [ee/libcglue] correct `__retarget_lock_acquire_recursive` race
2 parents efc828d + c9a9e2f commit f3c9ca0

1 file changed

Lines changed: 27 additions & 36 deletions

File tree

ee/libcglue/src/lock.c

Lines changed: 27 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -140,26 +140,26 @@ void __retarget_lock_acquire(_LOCK_T lock)
140140
#ifdef F___retarget_lock_acquire_recursive
141141
void __retarget_lock_acquire_recursive(_LOCK_T lock)
142142
{
143-
bool starting = false;
144143
int32_t thread_id = GetThreadId();
145-
starting = lock->count == 0;
146-
if (starting) {
147-
lock->thread_id = thread_id;
148-
}
149-
if (lock->thread_id == thread_id) {
150-
lock->count++;
151-
if (starting) {
152-
WaitSema(lock->sem_id);
153-
}
154-
} else {
155-
WaitSema(lock->sem_id);
156-
// Reached here means that the lock was acquired by another thread
157-
// so now we need to make it ours
158-
// We can't put the lock->count++ before the WaitSema because it will
159-
// cause a deadlock
160-
lock->thread_id = thread_id;
144+
145+
/* Recursive case: this thread already owns the gate. We can read
146+
* thread_id/count without the gate because only the owning thread
147+
* mutates them while it holds the gate, and only the owning thread
148+
* can take this branch. */
149+
if (lock->count > 0 && lock->thread_id == thread_id) {
161150
lock->count++;
151+
return;
162152
}
153+
154+
/* Otherwise acquire the gate for real. Only after WaitSema returns
155+
* do we touch thread_id/count — putting count++ before WaitSema is
156+
* what causes the classic recursive-mutex deadlock here, because a
157+
* higher-priority thread preempting between count++ and WaitSema
158+
* sees count>0, falls into the else branch, races past the sema, and
159+
* then nobody signals it after release. */
160+
WaitSema(lock->sem_id);
161+
lock->thread_id = thread_id;
162+
lock->count = 1;
163163
}
164164
#endif
165165

@@ -173,28 +173,19 @@ int __retarget_lock_try_acquire(_LOCK_T lock)
173173
#ifdef F___retarget_lock_try_acquire_recursive
174174
int __retarget_lock_try_acquire_recursive(_LOCK_T lock)
175175
{
176-
int res = 0;
177-
bool starting = false;
178176
int32_t thread_id = GetThreadId();
179-
starting = lock->count == 0;
180-
if (starting) {
181-
lock->thread_id = thread_id;
182-
}
183-
if (lock->thread_id == thread_id) {
184-
lock->count++;
185-
if (starting) {
186-
res = PollSema(lock->sem_id) > 0 ? 0 : 1;
187-
}
188-
} else {
189-
res = PollSema(lock->sem_id) > 0 ? 0 : 1;
190-
// Reached here means that the lock was acquired by another thread
191-
// so now we need to make it ours
192-
// We can't put the lock->count++ before the WaitSema because it will
193-
// cause a deadlock
194-
lock->thread_id = thread_id;
177+
178+
if (lock->count > 0 && lock->thread_id == thread_id) {
195179
lock->count++;
180+
return 0;
181+
}
182+
183+
if (PollSema(lock->sem_id) <= 0) {
184+
return 1; /* gate not free */
196185
}
197-
return res;
186+
lock->thread_id = thread_id;
187+
lock->count = 1;
188+
return 0;
198189
}
199190
#endif
200191

0 commit comments

Comments
 (0)