Repository navigation
Locks follow one rule: LLCoros where a coroutine reaches, LLMutex where only threads do - #591
Merged
Merged
Conversation
…re only threads do std::mutex, std::condition_variable and std::shared_mutex were picked site by site. Each lock now takes the type its takers call for: - A lock that main-thread code a coroutine can call takes -- llcorehttp's request, reply and handle locks and its stats, Script Studio's save history, recovery store, master index, job and analysis lanes and caches, text find and search, the syntax table names, parameter blocks, process launch, the avatar's joint map -- is an LLCoros::Mutex, with LLCoros::ConditionVariable where it is waited on. A coroutine that has to wait parks alone, and one that suspends holding it no longer leaves the next coroutine on the main thread waiting on its own thread. - A lock taken only by threads the code owns, or by the main loop's own frame path -- the watchdog's timeout, perf stats, the published joint numbers, the disabled GL work queue, the LSL reference compiler, the test fixtures -- is an LLMutex or LLCondition, whose assert catches a coroutine that reaches it. - std stays below the coroutine layer (LLSingleton, llerror and the log recorders, LLInstanceTracker, lockstatic, llthread's registry, llmutex itself) and in code that does not link llcommon (llwebrtc, dullahan). ALSharedMutex is LLSharedMutex's writer-preferring lock on its own, which neither side may re-enter; LLSharedMutex is built on it. LLStringTable takes it in place of std::shared_mutex, which under glibc let a reader that came while a writer waited in beside the first. Two races met on the way are gone. llprocess marked its job object made before it made it, so a second launch could read the handle unset or half configured; it is now made first, then marked with a release store that the unlocked check reads with acquire. The LSL service handed out its builtin names by reference to lists it cleared and refilled under a lock the caller no longer held; it hands out shared lists, made anew when the table changes. The Linux volume catcher keeps std::mutex: its cleanup() locks the child node mutex again through ChildNode::destroy() while it walks the set that destroy() erases from, which the recursive LLMutex would turn from a hang into a walk over a changing set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 9 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
RyeMutt
marked this pull request as ready for review
October 27, 2026 10:20
github-actions
Bot
added
c/cpp
llrender
llui
llcommon
cmake
llcorehttp
viewer
alscript
labels
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every
std::mutex,std::condition_variableandstd::shared_mutexnow takes the type its takers call for, instead of being picked site by site.The rule
LLCoros::Mutex/LLCoros::ConditionVariablewhere main-thread code that a coroutine can call takes the lock: llcorehttp's request, reply and handle locks andHTTPStats; Script Studio's save history, recovery store, master index, job and analysis lanes and caches; text find and search; the syntax table names; parameter blocks andALParamTypes; process launch;LLVOAvatarSelf's joint map. A coroutine that has to wait parks alone, and one that suspends while holding the lock no longer leaves the next coroutine waiting on its own thread.LLMutex/LLConditionwhere only threads the code owns, or the main loop's own frame path, take it: the watchdog's timeout,LLPerfStats, the published joint numbers, the disabled GL work queue, the LSL reference compiler, test fixtures.LLMutex's assert catches a coroutine that reaches one.LLSingleton, llerror and the log recorders,LLInstanceTracker, lockstatic, llthread's registry, llmutex itself) and in code that does not link llcommon (llwebrtc, dullahan).ALSharedMutex
LLSharedMutex's writer-preferring lock, now a class of its own (alsharedmutex.h) that neither side may re-enter.LLSharedMutexis built on it, andLLStringTabletakes it in place ofstd::shared_mutex. Under glibc,std::shared_mutexlet a reader that arrived while a writer waited in beside the first reader.Races fixed on the way
llprocessmarked its job object made before it made it, so a second launch could read the handle unset or half configured. It is now made first, then marked with a release store that the unlocked check reads with acquire.Left on std
The Linux PipeWire volume catcher: its
cleanup()locks the child node mutex again throughChildNode::destroy()while it walks the set thatdestroy()erases from. The recursiveLLMutexwould turn that hang into a walk over a changing set; it needs its own fix.Testing
vs2026-osRelWithDebInfo with tests on: the llcommon, llcorehttp, alscript, llui and newview test binaries and the viewer built with no warnings; 308/308 of those tests pass, plus the Python-peer llcommon tests (llleap,llprocess).alsharedmutex_testandllmutex_testpassed 25 and 10 runs in random order.alsharedmutex_test's late-reader case was run under glibc againststd::shared_mutex(fails) andALSharedMutex(passes).ENABLE_GL_WORK_QUEUEset.Not built here: macOS, Linux, and
lscript/allslreference.cpp(needs bison and flex).🤖 Generated with Claude Code