Remove CoreCLR PAL semaphore support - #132636
Conversation
Add a minipal read-write lock and use it for metadata synchronization. Remove the PAL semaphore implementation, exports, and tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 4 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @agocke |
There was a problem hiding this comment.
Pull request overview
This PR removes CoreCLR PAL “semaphore object” support (implementation, exports, constants, and PAL tests) and replaces metadata’s legacy UTSemReadWrite usage with a new minipal_rwlock abstraction (SRWLOCK on Windows, pthread_rwlock_t elsewhere, preferring writers when supported). The intent is to shrink the PAL synchronization surface while preserving metadata locking contracts and debug bookkeeping.
Changes:
- Add
minipal_rwlock(new minipal rwlock header + implementation, plus configure-time feature detection and build wiring). - Migrate metadata locking from
UTSemReadWritetominipal_rwlock, including updated helper APIs/macros and debug write-lock tracking. - Remove PAL semaphore object support and associated PAL semaphore tests, lists, exports, and error constants.
Reviewed changes
Copilot reviewed 45 out of 45 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/minipal/rwlock.h | Adds the minipal_rwlock public header (SRWLOCK/pthread-backed). |
| src/native/minipal/rwlock.c | Implements rwlock init/enter/leave for Windows and pthread platforms. |
| src/native/minipal/minipalconfig.h.in | Adds HAVE_PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP config define. |
| src/native/minipal/configure.cmake | Adds compile-time probe for writer-preference rwlock attribute support. |
| src/native/minipal/CMakeLists.txt | Includes rwlock.c in minipal sources. |
| src/coreclr/vm/gccover.cpp | Removes now-unused utsem.h include. |
| src/coreclr/utilcode/utsem.cpp | Deletes the legacy UTSemReadWrite implementation. |
| src/coreclr/utilcode/CMakeLists.txt | Removes utsem.cpp from utilcode build sources. |
| src/coreclr/pal/tests/palsuite/threading/WaitForSingleObject/WFSOSemaphoreTest/WFSOSemaphoreTest.cpp | Removes PAL semaphore WFSO test (PAL semaphore objects removed). |
| src/coreclr/pal/tests/palsuite/threading/releasesemaphore/test1/test.cpp | Removes PAL ReleaseSemaphore test (PAL semaphore objects removed). |
| src/coreclr/pal/tests/palsuite/threading/DuplicateHandle/test10/test10.cpp | Removes PAL DuplicateHandle(semaphore) test (PAL semaphore objects removed). |
| src/coreclr/pal/tests/palsuite/threading/CreateSemaphoreW_ReleaseSemaphore/test3/createsemaphore.cpp | Removes PAL CreateSemaphore/ReleaseSemaphore test (PAL semaphore objects removed). |
| src/coreclr/pal/tests/palsuite/threading/CreateSemaphoreW_ReleaseSemaphore/test2/CreateSemaphore.cpp | Removes PAL CreateSemaphore/ReleaseSemaphore test (PAL semaphore objects removed). |
| src/coreclr/pal/tests/palsuite/threading/CreateSemaphoreW_ReleaseSemaphore/test1/CreateSemaphore.cpp | Removes PAL CreateSemaphore/ReleaseSemaphore test (PAL semaphore objects removed). |
| src/coreclr/pal/tests/palsuite/paltestlist.txt | Removes semaphore-related entries from PAL test list. |
| src/coreclr/pal/tests/palsuite/paltestlist_to_be_reviewed.txt | Removes semaphore-related entry from “to be reviewed” list. |
| src/coreclr/pal/tests/palsuite/compilableTests.txt | Removes semaphore-related entries from compilable tests list. |
| src/coreclr/pal/tests/palsuite/CMakeLists.txt | Removes semaphore-related PAL test sources from build. |
| src/coreclr/pal/src/synchobj/semaphore.cpp | Deletes PAL semaphore object implementation. |
| src/coreclr/pal/src/synchmgr/wait.cpp | Removes semaphore from allowed wait/signalable PAL object types. |
| src/coreclr/pal/src/misc/errorstrings.cpp | Removes semaphore-timeout error string mapping. |
| src/coreclr/pal/src/include/pal/semaphore.hpp | Deletes PAL semaphore internal header. |
| src/coreclr/pal/src/include/pal/corunix.hpp | Removes otiSemaphore PAL object type id. |
| src/coreclr/pal/src/CMakeLists.txt | Removes semaphore object source from PAL build. |
| src/coreclr/pal/inc/pal.h | Removes PAL semaphore APIs and semaphore access constants from public PAL header. |
| src/coreclr/pal/inc/pal_error.h | Removes PAL-only semaphore-related error codes. |
| src/coreclr/md/runtime/mdinternalro.h | Updates reader-writer lock type to minipal_rwlock* in MDInternalRO contract. |
| src/coreclr/md/inc/rwutil.h | Introduces minipal_rwlock-based lock helpers and updates lock holder/macros. |
| src/coreclr/md/inc/metamodelrw.h | Replaces debug lock pointer tracking with explicit “write-locked” flag tracking. |
| src/coreclr/md/inc/mdinternalrw.h | Updates MDInternalRW reader-writer lock APIs to use minipal_rwlock*. |
| src/coreclr/md/enc/stdafx.h | Removes utsem.h include from metadata ENC build. |
| src/coreclr/md/enc/rwutil.cpp | Implements metadata lock helpers (CreateMDReadWriteLock, AcquireMD*Lock, etc.) and updates lock holder class. |
| src/coreclr/md/enc/metamodelrw.cpp | Updates debug write-lock assertion to use the new flag-based tracking. |
| src/coreclr/md/enc/mdinternalrw.cpp | Updates call sites to acquire/release metadata write lock using new helpers/types. |
| src/coreclr/md/compiler/stdafx.h | Removes utsem.h include from metadata compiler build. |
| src/coreclr/md/compiler/regmeta.h | Updates RegMeta lock type and lock wiring to minipal_rwlock*. |
| src/coreclr/md/compiler/regmeta.cpp | Updates RegMeta lock allocation/teardown and lock usage to minipal_rwlock helpers. |
| src/coreclr/md/compiler/mdutil.h | Updates global loaded-modules lock type to minipal_rwlock*. |
| src/coreclr/md/compiler/mdutil.cpp | Updates global loaded-modules lock initialization and lock usage to minipal_rwlock. |
| src/coreclr/inc/winwrap.h | Removes PAL-specific CreateSemaphore wrapper macro. |
| src/coreclr/inc/utsem.h | Deletes legacy UTSemReadWrite header. |
| src/coreclr/inc/metadata.h | Updates internal metadata interfaces to use minipal_rwlock* instead of UTSemReadWrite*. |
| src/coreclr/inc/corpriv.h | Updates IMetaDataHelper lock APIs to use minipal_rwlock*. |
| src/coreclr/inc/corhost.h | Removes local CreateSemaphore undef logic tied to PAL semaphore mapping. |
| src/coreclr/dlls/mscordac/mscordac_unixexports.src | Removes PAL semaphore exports from DAC unix export list. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 45 out of 45 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
src/coreclr/md/enc/rwutil.cpp:1294
- The comment block above CMDReadWriteLock::LockRead() says it obtains the "write lock", but this method acquires the read/shared lock. This is misleading when debugging lock-mode issues in metadata code.
This issue also appears on line 1318 of the same file.
// Used to obtain the write lock
src/coreclr/md/enc/rwutil.cpp:1318
- The comment block above CMDReadWriteLock::LockWrite() says it obtains the "read lock", but this method acquires the write/exclusive lock. Keeping these comment headers accurate helps avoid confusion when tracing lock ownership and contract violations.
// Used to obtain the read lock
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 46 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/native/minipal/configure.cmake:34
- The check for pthread_rwlockattr_setkind_np uses check_c_source_compiles(), which performs a link step. Because CMAKE_REQUIRED_LIBRARIES isn’t set to include the thread library, this probe can fail to link on platforms that require explicit pthread linkage (e.g., glibc), leaving HAVE_PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP unset even when the API is available.
| static Volatile<BOOL> g_fInitializedGlobalSystemInfo = FALSE; | ||
| SYSTEM_INFO g_SystemInfo; | ||
|
|
||
| static void InitializeGlobalSystemInfo() | ||
| { | ||
| if (!g_fInitializedGlobalSystemInfo) | ||
| { | ||
| GetSystemInfo(&g_SystemInfo); | ||
| g_fInitializedGlobalSystemInfo = TRUE; | ||
| } | ||
| } |
| check_c_source_compiles(" | ||
| #include <pthread.h> | ||
| int main(void) | ||
| { | ||
| pthread_rwlockattr_t attributes; |
CoreCLR no longer needs PAL semaphore objects: Unix
System.Threading.Semaphoreuses the managed wait subsystem, and metadata's legacyUTSemReadWritewas the remaining PAL consumer. Removing this support reduces the PAL synchronization surface and its associated object-management code.This change:
SRWLOCKon Windows andpthread_rwlock_telsewhere, requesting writer preference where supported;UTSemReadWriteusage while preserving lock contracts, debugger bookkeeping, ownership transfers, and debug assertions; andThe POSIX semaphores used by debugger startup coordination in
process.cppremain; they are independent of PAL semaphore objects.Testing
./build.sh clr+libs+host./build.sh clr.runtime+clr.paltests./build.sh clr.runtime -rc ReleasePerformance
BenchmarkDotNet comparison using baseline and changed Release CoreRun hosts, with 2 launches, 5 warmup iterations, and 15 measurement iterations of 250 ms:
Benchmark source
Note
This pull request description was created by GitHub Copilot.