Do not deadlock when the device compiler exits during a module build (#1393) - #1394
Closed
pvelesko wants to merge 2 commits into
Closed
Do not deadlock when the device compiler exits during a module build (#1393)#1394pvelesko wants to merge 2 commits into
pvelesko wants to merge 2 commits into
Conversation
A backend compiler that ends the process with exit() instead of reporting a rejected module (PoCL does this when the SPIR-V module needs an extension the target lacks, e.g. SPV_INTEL_function_pointers) makes chipStar hang: exit() runs the atexit handler on the same thread, which re-enters Device::DeviceVarMtx that Device::getOrCreateModule() is still holding across the build. The test interposes clBuildProgram and zeModuleCreate so the failure is reproduced without a driver that rejects the module, and bounds itself so it reports FAIL instead of burning the harness timeout.
…uild Device::getOrCreateModule() holds DeviceVarMtx across the backend compile. PoCL ends the process with exit() there when its SPIR-V reader rejects a module, for instance one that needs SPV_INTEL_function_pointers on a target without it. exit() runs chipStar's atexit handler on the same thread, and Device::deallocateDeviceVariables() then blocks forever on the lock the build still holds, so the process wedges with no diagnostic. Mark the module build region per thread and have CHIPUninitializeCallOnce() skip teardown when it is entered from inside one, reporting what happened. The rejected module now fails immediately with an explanation instead of hanging until the harness times out.
Collaborator
Author
|
Duplicate of #1389, which fixes the same self-deadlock in 5 lines by making DeviceVarMtx recursive. That is the better fix: teardown proceeds normally instead of being skipped. Closing in favour of it. |
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
1 participant
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.
Fixes #1393.
Device::getOrCreateModule()heldDeviceVarMtxacross the backend compile, so a driver callingstd::exit()there ran the atexit handler on the same thread anddeallocateDeviceVariables()blocked on the mutex the build still held. A thread local marker around the backend build letsCHIPUninitializeCallOnceskip teardown and log why, so the process exits with the compiler code instead of hanging.Regression test added: it interposes
clBuildProgramandzeModuleCreateso libCHIP dispatches into them, then forks and re execs so the parent bounds the child rather than burning the harness timeout. Before the fix it reports the deadlock after 45.8 s; after, it passes in 1.6 s. Skips cleanly where the interposer cannot fire.Trade off worth noting: this path skips teardown, so the process exits with the compiler exit code and leaks whatever the runtime held. That is acceptable since the process is already dying, but it is deliberate rather than incidental.
Regression runs: dgpu opencl 1014/1014, dgpu level0 1045/1046, the single failure being
TestStaticLibRDCwhich passes standalone in 75.9 s and timed out only under concurrent load.Upstream causes are filed separately at pocl/pocl#2258 and KhronosGroup/SPIRV-LLVM-Translator#3939.