#2115 RexxQueryMacro is called with the kernel lock held: while rxapi answers, every thread stops

[Josep Maria Blasco]

Verified against trunk r13268 (native, Linux x86_64); the two files are unchanged up to r13271.

Summary: two places in the interpreter ask rxapi whether a name is in
the macrospace while holding the kernel lock:

• RexxActivation::callMacroSpaceFunction() — RexxQueryMacro(macroName, &position), on every external call that is not resolved before the
  macrospace is searched: not only a routine that does not exist, but every
  call of a rexxutil function (call SysSleep 0 asks rxapi each time, not
  just the first: with rxapi stopped, the third SysSleep 0 in a row also
  waits for it);
• PackageManager::loadRequires() — RexxQueryMacro(shortName->getStringData(), &macroPosition), on every ::REQUIRES / .Package~loadPackage of a
  package that is not loaded yet.

Each is a round trip to the rxapi daemon. While it lasts, no other Rexx
thread of the process can run. The other rxapi calls the interpreter makes
from the same paths already release the lock — getMacroCode()
(RexxResolveMacroFunction), PackageManager around
RexxRegisterFunctionDll and RexxResolveRoutine (UnsafeBlock releaser; // don't hold the block while making the API call) — and so do all the
rxapi calls made from native methods (queued(), the .RexxQueue methods), since
native code runs without the lock. These two seem to have been missed.

Normally rxapi answers quickly and nobody notices. But if rxapi is slow (busy
with another client, being started, paging, stopped in a debugger), every
thread of every Rexx process that does an external call or loads a package
stops with it, not just the thread that asked. And an embedder whose rxapi
traffic depends on the host's own threads can deadlock: we found it in the
ooRexx/WASM port under Node, where the daemon's replies arrive through the
JavaScript event loop; when JavaScript calls a Rexx method synchronously and
another Rexx thread is waiting for rxapi with the lock held, the method
waits for the lock, the lock's holder waits for the event loop, and the
event loop waits for the method.

Reproduction: query-macro-locked.rex (attached; plain Rexx, needs
pkill): a second thread counts as fast as it can. The main thread measures
its rate for one second, then freezes rxapi for 3 seconds (pkill -STOP -x rxapi, and pkill -CONT from a background shell) and calls a routine that
does not exist (argument external) or loads a package (argument
requires). It prints how much the other thread counted during the call,
as a percentage of its rate before (about 0 if it was stopped; over 100 if
it ran, as it then has the interpreter to itself).

trunk r13268:           external 7     requires 0
trunk r13268 + diff:    external 281   requires 297

For comparison, the same wait for rxapi inside queued() (a native method)
does not stop the other thread, with or without the diff.

(Careful: pkill -STOP -x rxapi stops the rxapi of every ooRexx process of
that user for those 3 seconds.)

Fix (attached diff): the same UnsafeBlock around the two
RexxQueryMacro calls that getMacroCode() already has around
RexxResolveMacroFunction (same name strings, held by the caller as there).
The full test suite (test/trunk r13268, native, 402 groups, 24283 tests)
gives the same totals with and without it (396 groups ok, 10 failures in
the 6 others, which fail here for reasons of their own); two groups differed
once, in opposite directions (ProcessInvocation failed once with the diff,
TRACE once without it), and both pass 5 of 5 runs with either build: timing.

LanguageParser::processInstore() and restoreFromMacroSpace() call
RexxQueryMacro / RexxResolveMacroFunction the same way; they were not
part of what we tested (they run for RexxStart with an empty instore), so
they are not in the diff, but they likely deserve the same treatment.