Avoid recursive cache lock in SDL TTF ports - #27590
Conversation
When building port libraries, build_port compiles sources with emcc in a subprocess while the parent process holds EM_CACHE_IS_LOCKED. Passing -sUSE_* flags causes the child emcc to resolve ports and attempt to acquire the cache lock if dependencies like harfbuzz are not yet in the cache, causing an AssertionError. Pass include directories directly instead of port settings flags when compiling SDL TTF port sources. Also ensure parent directories exist when writing port assets.
| self.set_setting('USE_SDL', 2) | ||
| self.set_setting('USE_SDL_TTF', 2) | ||
| with shared.cache.lock('test_sdl_ttf_parent_cache_lock'): | ||
| ports.get_port_by_name('sdl2_ttf').get(ports.Ports, settings, shared) |
There was a problem hiding this comment.
I'd rather find a way to test this from outside using embuilder and/or emcc rather then using the internal API like this.
There was a problem hiding this comment.
I don't see a way to deterministically test this. Do you have any suggestions?
|
In other words, the assertion in question should never fire. At least when when N process are all trying to build the same ports. The only time this assertion should ever fire is if another process deletes a ports right after the current process builds it. But being robust against the deletion of ports is not something I think we can ever really be robust against, and deleting ports while other folks are using the compiler seems like a situation that lead to crash like this. The question is, how is it possible that the second process is deleting/clearing libsdl2 here: emscripten/tools/ports/__init__.py Line 428 in 0dc8f19 |
|
My understanding with two workers:
The why |
Yes, I think that analysis matches the on in #27160 (comment) ? I also don't see how its possible for The logical conclusion seems to that we cannot use filesystem locks files to enforce sequential consistency between processes.. which I find hard to believe, and also very worrying since I don't know how else to solve this problem if we cannot depend on the lock files for synchronization like this. |
|
This change works around the issue, but its doesn't solve it. Maybe that is good enough for now? |
|
I think this issue has been seen now on mac, linux and windows bots, but I've never been able to reproduce it locally, even using Have you had any luck reproing locally? |
I don't think the issue is filesystem locks. It's that we have multiple locks that can be interleaved and there are state checks outside of locks. I think we can fix this by moving some of the locks to cover more of the process or avoid trying to lock in subprocesses (what this PR does and not use |
The logic of the ports system should already guarantee that no locks are ever needed during subprocess. That we why we have the
I believe all the analysis (both yours, and mine, and geminis) points to some kind of bug at phase 4 where process B gets the lock but does not see the library as stamp file that process A wrote. This should be impossible because the stamp file was written (and the file closed) by process A before it released the FS lock. |
|
I'm also not convinced this patch would actually the fix the issue, but maybe just move it around. The real issue here is that after Process A is done building a library, another parallel Process B sees it as not up-to-date and deletes it. This means that all the child processes of Process A could be run without the library present (since it was delete by process B). Maybe this doesn't matter as much because then Process B deletes the library I think it just deletes the In other words I think this change might mask the issue, but I don't think it solves it. The real issue is that sibling processes are deleting libraries that the other one just built.. |
The only way I've been able to reproduce is adding sleeps and some special bypasses: |
2 participants
When building port libraries, build_port compiles sources with emcc in a subprocess while the parent process holds EM_CACHE_IS_LOCKED. Passing -sUSE_* flags causes the child emcc to resolve ports and attempt to acquire the cache lock if dependencies like harfbuzz are not yet in the cache, causing an AssertionError.
Pass include directories directly instead of port settings flags when compiling SDL TTF port sources. Also ensure parent directories exist when writing port assets.