#2071 SysMutex::request() can silently fail to lock; every caller discards the result

[Moritz Hoffmann]

This one is potentially serious, and I believe it is the mechanism behind [bugs:#2020].

SysMutex::request() returns false without locking when the mutex is not in the created
state, and every caller throws the result away:

common/platform/unix/SysSemaphore.hpp:84
inline bool request() { if (!created) return false; return pthread_mutex_lock(&mutexMutex) == 0; }

common/platform/unix/SysSemaphore.cpp:406
void SysMutex::close() { if (created) { pthread_mutex_destroy(&mutexMutex); created = false; } }

interpreter/runtime/Interpreter.hpp:77-78
static inline void getDispatchLock() { dispatchLock.request(); } // return value discarded
static inline void releaseDispatchLock() { dispatchLock.release(); }

ResourceSection / DispatchSection call these from their constructors and ignore the outcome.
So if a mutex is not yet created, or has already been close()d, the guard constructs
successfully, the caller enters the critical section, and there is no mutual exclusion at all —
with nothing anywhere able to notice.

Interpreter::processShutdown() (Interpreter.cpp:156-159) calls closeLocks(), which destroys
both dispatchLock and resourceLock. Any thread still running past that point silently loses
all locking.

Evidence: I instrumented waitingActivities with an in-memory ring buffer recording, per
mutation, the thread id, the deque size, and a per-thread count of held DispatchSections, then
read it out of a core dump. Immediately before the corruption:

seq | thread | op | qsize | lockDepth
283 | B | push_back | 1 | 1
284 | A | pop_front | 5745943456555 | 1 <- deque state destroyed

Two different threads mutating the deque back-to-back, each believing it holds the dispatch lock.
A sequential push-then-pop cannot corrupt a std::deque, so these were concurrent — which is only
possible if DispatchSection was not excluding them.

Suggested fixes:

1. Treat a failed request() as a hard error rather than silently continuing.
2. Do not destroy the locks while threads may still run.

Platform: Linux x86_64, gcc, current trunk.

[Moritz Hoffmann]

Correction to my own report: the closeLocks() pathway I described cannot occur on Unix, so
please disregard that part.

Interpreter::processShutdown() is never reached on Unix, because of a separate copy-paste bug:

interpreter/platform/unix/SystemInterpreter.cpp:59
void SystemInterpreter::processShutdown()
{
// now do the platform independent shutdown
Interpreter::processStartup(); // <-- calls processStartup, not processShutdown
}

(Windows is correct — interpreter/platform/windows/SystemInterpreter.cpp:182 calls
Interpreter::processShutdown().) Filed separately.

The locks are created by _rexx_init() (a library constructor, unix/SystemInitialization.cpp:52)
and are therefore never destroyed on Unix. So "a thread outlives closeLocks() and silently loses
locking" is not a reachable scenario there.

What still stands in this ticket: SysMutex::request() returning false without locking when
!created, while getDispatchLock() / getResourceLock() discard the return value, remains a real
defect. A lock that can silently fail to lock, with no way for a caller to detect it, is worth
fixing on its own merits.

What I no longer have: a confirmed mechanism for the concurrency evidence that motivated this
report. The instrumentation still shows two threads mutating waitingActivities back-to-back while
each reports holding a DispatchSection:

seq | thread | op | qsize | lockDepth
283 | B | push_back | 1 | 1
284 | A | pop_front | 5745943456555 | 1 <- deque state destroyed

That observation is solid, but with the closeLocks route ruled out I cannot currently explain how
both threads were inside the critical section. Treat this ticket as "unsafe API worth fixing" rather
than "diagnosed cause of [bugs:#2020]" until someone pins that down.

[Moritz Hoffmann]

Fixed in r13182.

[jfaucher]

Commit [r13182]