Skip to content

Commit aa05bc8

Browse files
committed
mpid/thread: Fix thread unsafe recursive lock check
We need to read the mutex owner value after we acquire the lock or else the read may conflict with an update whichever thread holds the lock. Race detected by ThreadSanitizer.
1 parent d0c8846 commit aa05bc8

File tree

1 file changed

+9
-12
lines changed

1 file changed

+9
-12
lines changed

src/mpid/common/thread/mpidu_thread_fallback.h

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -139,21 +139,18 @@ M*/
139139
if (MPIR_ThreadInfo.isThreaded) { \
140140
int equal_ = 0; \
141141
MPL_thread_id_t self_, owner_; \
142+
int err_ = 0; \
143+
MPL_DBG_MSG_P(MPIR_DBG_THREAD,VERBOSE,"enter MPIDU_Thread_mutex_lock %p", &mutex); \
144+
MPIDU_Thread_mutex_lock(&mutex, &err_, MPL_THREAD_PRIO_HIGH); \
145+
MPL_DBG_MSG_P(MPIR_DBG_THREAD,VERBOSE,"exit MPIDU_Thread_mutex_lock %p", &mutex); \
142146
MPL_thread_self(&self_); \
143147
owner_ = mutex.owner; \
144148
MPL_thread_same(&self_, &owner_, &equal_); \
145-
if (!equal_) { \
146-
int err_ = 0; \
147-
MPL_DBG_MSG_P(MPIR_DBG_THREAD,VERBOSE,"enter MPIDU_Thread_mutex_lock %p", &mutex); \
148-
MPIDU_Thread_mutex_lock(&mutex, &err_, MPL_THREAD_PRIO_HIGH);\
149-
MPL_DBG_MSG_P(MPIR_DBG_THREAD,VERBOSE,"exit MPIDU_Thread_mutex_lock %p", &mutex); \
150-
MPIR_Assert(err_ == 0); \
151-
MPIR_Assert(mutex.count == 0); \
152-
MPL_thread_self(&mutex.owner); \
153-
} else { \
154-
/* assert all recursive usage */ \
155-
MPIR_Assert(0); \
156-
} \
149+
MPIR_Assert(err_ == 0); \
150+
/* FIXME: owner and count checks are the same? */ \
151+
MPIR_Assert(!equal_); \
152+
MPIR_Assert(mutex.count == 0); \
153+
MPL_thread_self(&mutex.owner); \
157154
mutex.count++; \
158155
} \
159156
} while (0)

0 commit comments

Comments
 (0)