mirror of
https://github.com/hrydgard/ppsspp.git
synced 2026-10-01 14:58:14 +00:00
MemBlockInfo: fix double-lock deadlock in FindWriteTagByFlag (previous commit)
FindWriteTagByFlag(flush=false) is only called from FormatMemWriteTagAtNoFlush(), which is itself only called from within FlushPendingMemInfo() - which already holds pendingReadMutex for its entire body. The previous commit added an unconditional lock of that same (non-recursive) mutex here, so any path that reaches a memory write's tag formatting while a flush is in progress double-locks it and hangs/crashes. Repro: PPSSPPHeadless --graphics=software on pspautotests/tests/gpu/clipping/homogeneous.prx reliably hit this. Only take the lock when flush=true (i.e. when we're not already guaranteed to be called from inside FlushPendingMemInfo's locked section), using a defer_lock so the two call sites stay consistent. Verified fixed: 4/4 clean runs of the repro above, plus the full UnitTest suite (49/49) still passes.
This commit is contained in:
1 parent
527c50d32e
commit
9afab39bc8
1 file changed
+11
-5
@@ -639,17 +639,23 @@ std::vector<MemBlockInfo> FindMemInfoByFlag(MemBlockFlags flags, uint32_t start,
|
||||
static const char *FindWriteTagByFlag(MemBlockFlags flags, uint32_t start, uint32_t size, size_t *tagLen, bool flush = true) {
|
||||
start = NormalizeAddress(start);
|
||||
|
||||
// See the comment in FindMemInfo() above. Note: the returned tag pointer is
|
||||
// only valid until the next Mark() call per FastFindWriteTag()'s own contract,
|
||||
// so callers must treat it as transient exactly as they already do.
|
||||
//
|
||||
// flush=false means we're being called from FormatMemWriteTagAtNoFlush(), which
|
||||
// is only ever called from within FlushPendingMemInfo() - which already holds
|
||||
// pendingReadMutex for its whole body. Locking it again here would deadlock (or
|
||||
// be undefined behavior, since it's a plain non-recursive std::mutex), so only
|
||||
// take the lock ourselves when we might not already be holding it.
|
||||
std::unique_lock<std::mutex> guard(pendingReadMutex, std::defer_lock);
|
||||
if (flush) {
|
||||
if (pendingNotifyMinAddr1 < start + size && pendingNotifyMaxAddr1 >= start)
|
||||
FlushPendingMemInfo();
|
||||
if (pendingNotifyMinAddr2 < start + size && pendingNotifyMaxAddr2 >= start)
|
||||
FlushPendingMemInfo();
|
||||
guard.lock();
|
||||
}
|
||||
|
||||
// See the comment in FindMemInfo() above. Note: the returned tag pointer is
|
||||
// only valid until the next Mark() call per FastFindWriteTag()'s own contract,
|
||||
// so callers must treat it as transient exactly as they already do.
|
||||
std::lock_guard<std::mutex> guard(pendingReadMutex);
|
||||
if (flags & MemBlockFlags::ALLOC) {
|
||||
const char *tag = allocMap.FastFindWriteTag(MemBlockFlags::ALLOC, start, size, tagLen);
|
||||
if (tag)
|
||||
|
||||
Reference in new issue
Block a user