#2103 an ending thread uses its Activity after it may have been swept

[Josep Maria Blasco]

Description:

When a thread finishes and the pool of idle activities is full,
ActivityManager::poolActivity() (interpreter/concurrency/ActivityManager.cpp,
trunk r13263) calls cleanupActivityResources(), removes the activity from
allActivities() and returns false. Activity::runThread() then releases the
kernel lock and calls ActivityManager::activityEnded(), which takes the
resource lock and uses the activity again (cleanupActivityResources() a
second time).

allActivities() is what keeps an Activity alive for the garbage collector
(ActivityManager::live()). Between poolActivity() and activityEnded() the
ending thread holds no lock and its Activity is not anchored anywhere, so a
collection on another thread can sweep it and reuse its memory;
activityEnded() then works on whatever object is there now.

Seen as a rare segmentation fault at exit in a multi-threaded program (our
ooRexx-Python bridge's stress test: about 1 run in 700). In the core, the
crashing thread is in threadFnc → ActivityManager::activityEnded →
NativeActivation::clearLocalReferences() with this = 0xffffffffffffffff
(Activity::getApiContext(), i.e. topStackFrame). The memory where that
thread's Activity was holds a StringHashContents (its vtable), and the -1
read as topStackFrame is that object's NoMore. The main thread was in
Interpreter::terminateInterpreter() → ~InstanceBlock →
InterpreterInstance::terminate(), just after collectAndUninit(), waiting for
the resource lock held by the crashing thread.

Reproducer (attached, activity-ended.rex, pure Rexx): rounds of 7
threads, 6 to fill the pool and a 7th that ends a little later, alone, through
activityEnded(); the Messages returned by start() are not kept (a kept
Message also anchors the Activity); one more thread allocates all the time.
The natural window is a few instructions wide, so the attached
activity-ended-window.diff (debug instrumentation only) widens it:
ORX_AE_WINDOW_US=20000 sleeps 20 ms at the start of activityEnded(),
before the resource lock, and then aborts with "AE-SWEPT" if the activity's
vtable pointer has changed. ORX_AE_ORDER_FIXED=1 applies the fix below.
Linux x86_64, rexx activity-ended.rex 100:

ORX_AE_WINDOW_US=20000                        AE-SWEPT in 10 of 10 runs
ORX_AE_WINDOW_US=20000 ORX_AE_ORDER_FIXED=1   0 of 10 (all completed)
no window (either way)                        0 of 5

Fix (attached, activity-ended-fix.diff): when poolActivity() does not
pool the activity, it only returns false, and leaves the removal and the
cleanup to activityEnded(), which runThread() always calls next and which
does both under the resource lock while the activity is still anchored (the
collector's marking takes the resource lock too). poolActivity()'s only
caller is InterpreterInstance::poolActivity(), whose only caller is
runThread(). It also avoids calling cleanupActivityResources() twice.

ooRexx test suite (test/trunk, -X native_api, 23,379 tests) with the fix:
9 failures and 1 error, all of which also fail here without it (files, stdin,
time of day).

Possibly the same cause: a hang at exit we saw twice in the same stress test,
with a thread in activityEnded() → cleanupActivityResources() →
pthread_cond_destroy() never returning and the main thread waiting for the
resource lock in terminateInterpreter(). Not verified.

[Erich]

Committed the code patch with revision [r13265]