+26 -9

runtime/caml/platform.h

··· 113 113 114 114 #ifdef _WIN32 115 115 116 116 - typedef SRWLOCK caml_plat_mutex; 117 117 - #define CAML_PLAT_MUTEX_INITIALIZER SRWLOCK_INIT 116 116 + typedef struct { 117 117 + SRWLOCK lock; 118 118 + _Atomic DWORD owner_tid; /* 0 if not owned */ 119 119 + /* The "owner_tid" field is not always protected by "lock"; it is 120 120 + * also accessed without holding "lock". */ 121 121 + } caml_plat_mutex; 122 122 + #define CAML_PLAT_MUTEX_INITIALIZER { SRWLOCK_INIT, 0 } 118 123 119 124 typedef CONDITION_VARIABLE caml_plat_cond; 120 125 #define CAML_PLAT_COND_INITIALIZER CONDITION_VARIABLE_INIT ··· 622 627 623 628 Caml_inline void caml_plat_lock_blocking(caml_plat_mutex* m) 624 629 { 625 625 - AcquireSRWLockExclusive(m); 626 626 - DEBUG_LOCK(m); 630 630 + DWORD self_tid = GetCurrentThreadId(); 631 631 + if (m->owner_tid != self_tid) { 632 632 + AcquireSRWLockExclusive(&m->lock); 633 633 + m->owner_tid = self_tid; 634 634 + DEBUG_LOCK(m); 635 635 + } else { 636 636 + check_err("lock", EDEADLK); 637 637 + } 627 638 } 628 639 629 640 Caml_inline int caml_plat_try_lock(caml_plat_mutex* m) 630 641 { 631 631 - BOOLEAN rc = TryAcquireSRWLockExclusive(m); 632 632 - if (!rc) { 633 633 - return 0; 634 634 - } else { 642 642 + if (TryAcquireSRWLockExclusive(&m->lock)) { 643 643 + m->owner_tid = GetCurrentThreadId(); 635 644 DEBUG_LOCK(m); 636 645 return 1; 646 646 + } else { 647 647 + return 0; 637 648 } 638 649 } 639 650 640 651 Caml_inline void caml_plat_unlock(caml_plat_mutex* m) 641 652 { 653 653 + DWORD self_tid = GetCurrentThreadId(); 642 654 DEBUG_UNLOCK(m); 643 643 - ReleaseSRWLockExclusive(m); 655 655 + if (m->owner_tid == self_tid) { 656 656 + m->owner_tid = 0; 657 657 + ReleaseSRWLockExclusive(&m->lock); 658 658 + } else { 659 659 + check_err("unlock", EPERM); 660 660 + } 644 661 } 645 662 646 663 #else

+40 -12

runtime/sync_win32.h

··· 33 33 34 34 Caml_inline int sync_mutex_create(sync_mutex * res) 35 35 { 36 36 - sync_mutex m = caml_stat_alloc_noexc(sizeof(SRWLOCK)); 36 36 + sync_mutex m = caml_stat_alloc_noexc(sizeof(caml_plat_mutex)); 37 37 if (m == NULL) return ENOMEM; 38 38 - InitializeSRWLock(m); 38 38 + InitializeSRWLock(&m->lock); 39 39 + m->owner_tid = 0; 39 40 *res = m; 40 41 return 0; 41 42 } ··· 48 49 49 50 Caml_inline int sync_mutex_lock(sync_mutex m) 50 51 { 51 51 - AcquireSRWLockExclusive(m); 52 52 - return 0; 52 52 + DWORD self_tid = GetCurrentThreadId(); 53 53 + if (m->owner_tid != self_tid) { 54 54 + AcquireSRWLockExclusive(&m->lock); 55 55 + m->owner_tid = self_tid; 56 56 + return 0; 57 57 + } else { 58 58 + return EDEADLK; 59 59 + } 53 60 } 54 61 55 62 #define MUTEX_PREVIOUSLY_UNLOCKED 0 ··· 57 64 58 65 Caml_inline int sync_mutex_trylock(sync_mutex m) 59 66 { 60 60 - return TryAcquireSRWLockExclusive(m) ? 61 61 - MUTEX_PREVIOUSLY_UNLOCKED : MUTEX_ALREADY_LOCKED; 67 67 + if (TryAcquireSRWLockExclusive(&m->lock)) { 68 68 + m->owner_tid = GetCurrentThreadId(); 69 69 + return MUTEX_PREVIOUSLY_UNLOCKED; 70 70 + } else { 71 71 + return MUTEX_ALREADY_LOCKED; 72 72 + } 62 73 } 63 74 64 75 Caml_inline int sync_mutex_unlock(sync_mutex m) 65 76 { 66 66 - ReleaseSRWLockExclusive(m); 67 67 - return 0; 77 77 + DWORD self_tid = GetCurrentThreadId(); 78 78 + if (m->owner_tid == self_tid) { 79 79 + m->owner_tid = 0; 80 80 + ReleaseSRWLockExclusive(&m->lock); 81 81 + return 0; 82 82 + } else { 83 83 + return EPERM; 84 84 + } 68 85 } 69 86 70 87 /* Condition variables */ ··· 98 115 99 116 Caml_inline int sync_condvar_wait(sync_condvar c, sync_mutex m) 100 117 { 101 101 - if (!SleepConditionVariableSRW(c, m, INFINITE, 0 /* exclusive */)) { 102 102 - int rc = caml_posixerr_of_win32err(GetLastError()); 103 103 - return rc == 0 ? EINVAL : rc; 118 118 + DWORD self_tid = GetCurrentThreadId(); 119 119 + int rc = 0; 120 120 + if (m->owner_tid == self_tid) { 121 121 + m->owner_tid = 0; 122 122 + if (SleepConditionVariableSRW(c, &m->lock, INFINITE, 123 123 + 0 /* exclusive */)) { 124 124 + m->owner_tid = self_tid; 125 125 + } else { 126 126 + rc = caml_posixerr_of_win32err(GetLastError()); 127 127 + /* Not clear if the thread owns the mutex or not, but there's a 128 128 + * fatal error anyway. */ 129 129 + } 130 130 + } else { 131 131 + rc = EPERM; 104 132 } 105 105 - return 0; 133 133 + return rc; 106 134 } 107 135 108 136 #endif /* CAML_SYNC_WIN32_H */

+26 -5

runtime/win32.c

··· 1435 1435 1436 1436 CAMLexport void caml_plat_mutex_init(caml_plat_mutex * m) 1437 1437 { 1438 1438 - InitializeSRWLock(m); 1438 1438 + InitializeSRWLock(&m->lock); 1439 1439 + m->owner_tid = 0; 1439 1440 } 1440 1441 1441 1442 void caml_plat_assert_locked(caml_plat_mutex *m) 1442 1443 { 1443 1444 #ifdef DEBUG 1444 1444 - BOOLEAN r = TryAcquireSRWLockExclusive(m); 1445 1445 + BOOLEAN r = TryAcquireSRWLockExclusive(&m->lock); 1445 1446 if (r == 0) { 1446 1447 /* ok, it was locked */ 1447 1448 return; ··· 1455 1456 { 1456 1457 /* Avoid exceptions */ 1457 1458 caml_enter_blocking_section_no_pending(); 1458 1458 - AcquireSRWLockExclusive(m); 1459 1459 + DWORD self_tid = GetCurrentThreadId(); 1460 1460 + if (m->owner_tid != self_tid) { 1461 1461 + AcquireSRWLockExclusive(&m->lock); 1462 1462 + m->owner_tid = self_tid; 1463 1463 + } else { 1464 1464 + check_err("lock_non_blocking", EDEADLK); 1465 1465 + } 1459 1466 caml_leave_blocking_section(); 1460 1467 DEBUG_LOCK(m); 1461 1468 } ··· 1475 1482 void caml_plat_wait(caml_plat_cond *cond, caml_plat_mutex* mut) 1476 1483 { 1477 1484 caml_plat_assert_locked(mut); 1478 1478 - BOOL rc = SleepConditionVariableSRW(cond, mut, INFINITE, 0 /* exclusive */); 1479 1479 - check_err("wait", rc == 0); 1485 1485 + DWORD self_tid = GetCurrentThreadId(); 1486 1486 + int rc = 0; 1487 1487 + if (mut->owner_tid == self_tid) { 1488 1488 + mut->owner_tid = 0; 1489 1489 + if (SleepConditionVariableSRW(cond, &mut->lock, INFINITE, 1490 1490 + 0 /* exclusive */)) { 1491 1491 + mut->owner_tid = self_tid; 1492 1492 + } else { 1493 1493 + rc = caml_posixerr_of_win32err(GetLastError()); 1494 1494 + /* Not clear if the thread owns the mutex or not, but there's a 1495 1495 + * fatal error anyway. */ 1496 1496 + } 1497 1497 + } else { 1498 1498 + rc = EPERM; 1499 1499 + } 1500 1500 + check_err("wait", rc); 1480 1501 } 1481 1502 1482 1503 void caml_plat_broadcast(caml_plat_cond* cond)

+5 -14

stdlib/mutex.mli

··· 38 38 by another thread will suspend until the other thread unlocks 39 39 the mutex. 40 40 41 41 - @raise Sys_error if the mutex is already locked by the thread 42 42 - calling {!Mutex.lock} on Unix platforms. On Windows, recursive 43 43 - locking deadlocks. 41 41 + @raise Sys_error if the mutex is already locked by the thread calling 42 42 + {!Mutex.lock}. 44 43 45 44 @before 4.12 {!Sys_error} was not raised for recursive locking 46 46 - (platform-dependent behaviour). 47 47 - 48 48 - @since 5.3 On Windows, {!Sys_error} is not raised for recursive 49 49 - locking. *) 45 45 + (platform-dependent behaviour) *) 50 46 51 47 val try_lock : t -> bool 52 48 (** Same as {!Mutex.lock}, but does not suspend the calling thread if ··· 58 54 (** Unlock the given mutex. Other threads suspended trying to lock 59 55 the mutex will restart. The mutex must have been previously locked 60 56 by the thread that calls {!Mutex.unlock}. 61 61 - 62 62 - @raise Sys_error if the mutex is unlocked or was locked by another 63 63 - thread. This doesn't apply on Windows since OCaml 5.3. 57 57 + @raise Sys_error if the mutex is unlocked or was locked by another thread. 64 58 65 59 @before 4.12 {!Sys_error} was not raised when unlocking an unlocked mutex 66 66 - or when unlocking a mutex from a different thread. 67 67 - 68 68 - @since 5.3 On Windows, {!Sys_error} is not raised if the mutex is 69 69 - unlocked or was locked by another thread. *) 60 60 + or when unlocking a mutex from a different thread. *) 70 61 71 62 val protect : t -> (unit -> 'a) -> 'a 72 63 (** [protect mutex f] runs [f()] in a critical section where [mutex]

-1

testsuite/tests/lib-threads/mutex_errors.ml

··· 2 2 include systhreads; 3 3 hassysthreads; 4 4 no-tsan; (* tsan detects the mutex errors and fails *) 5 5 - not-windows; (* Windows' SRWLock differ from pthreads ERRORCHECK mutexes. *) 6 5 { 7 6 bytecode; 8 7 }{