Skip to content

Commit 8cd8e34

Browse files
committed
fix(jit): release recorder token before native parks
1 parent 709f671 commit 8cd8e34

5 files changed

Lines changed: 234 additions & 5 deletions

File tree

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
## Recorder-token ownership at blocking native parks
2+
3+
The b1.2.0 heavy threaded-flush gate exposed a recorder-token liveness cycle.
4+
A main TG entered `threading.thread:join()` while it still owned the global JIT
5+
recorder token. A child running `jit.flush()` asynchronously changed the
6+
recorder state from active `LJ_TRACE_RECORD` to aborted `LJ_TRACE_RECORD`, then
7+
waited to acquire that token. The owner was already parked waiting for the
8+
child, and only the owner could finish recorder cleanup and release the token:
9+
10+
```
11+
main: thread:join -> native futex park (owns jit_token)
12+
child: jit.flush -> lj_jit_token_acquire_wait (must finish before exit)
13+
```
14+
15+
This was not a slow stress case. The exact heavy case failed twice at its
16+
60-second join timeout; GDB showed `jit_token == main_tid`, `J->L == main`,
17+
`J->state == LJ_TRACE_RECORD` with `LJ_TRACE_ACTIVE` cleared, the main TG in
18+
`threading_futex_wait_l()`, and the sole live child spinning in
19+
`lj_jit_token_acquire_wait()`.
20+
21+
### Rule
22+
23+
A Lua/TG owner must not enter a potentially blocking native park while it owns
24+
unpublished recorder state. Immediately before the actual park it calls
25+
`lj_trace_abort_owner()`, which discards that owner's unpublished trace state
26+
and releases the token. Published traces are unaffected.
27+
28+
The threading library applies the rule in its common futex-wait wrapper, which
29+
covers contended join, spawn/activation, mutex, and lifecycle waits.
30+
31+
The rule is kept at concrete blocking sites instead of in
32+
`lj_native_enter_l()`. That helper publishes a native stack snapshot and is not
33+
itself a promise that its caller will block; making every future native-entry
34+
caller abort would unnecessarily constrain nonblocking C paths.
35+
36+
### Channel audit and reentrant recorder boundary
37+
38+
Channels have a separate futex substrate in `lj_chan.c`, so ordinary blocking
39+
and timed channel waits apply the same pre-park owner teardown after their
40+
optimistic spin fails. Try operations, zero-timeout operations, and handoffs
41+
completed during the spin remain untouched.
42+
43+
A direct, unguarded application of `lj_trace_abort_owner()` was tested and
44+
rejected: a trace `start` event callback can block while `trace_state()` is
45+
still on the C stack. Tearing down `J->cur` reentrantly from that callback
46+
caused an immediate production-build segmentation fault. This applies equally
47+
to channel and threading-library parks.
48+
49+
Each concrete park therefore compares the current TG id with
50+
`vmevent_owner_acq(g)`. The exact VM-event callback owner keeps the token and
51+
lets the outer recorder frame perform cleanup after it unwinds; all ordinary
52+
parks abort and release before sleeping. The guard is deliberately not folded
53+
into `lj_trace_abort_owner()`, whose detach/teardown callers have different
54+
lifetime obligations.
55+
56+
The safe follow-up boundary is one of:
57+
58+
- make contended JIT control logically invalidate/queue work without waiting
59+
for a token held by the callback; or
60+
- publish an explicit recorder-callback/reentrancy state, request abort, avoid
61+
the park, and finish cleanup only after `trace_state()` unwinds.
62+
63+
The guarded callback still cannot synchronously wait for a peer whose
64+
`jit.flush()` must acquire its token. A bounded channel receive or timed join
65+
returns first, after which recorder unwind lets the peer finish. Making that
66+
cross-TG dependency synchronous requires the broader nonblocking JIT-control
67+
protocol above and remains b1.2.1 debt.
68+
69+
### Validation
70+
71+
- The pre-fix heavy case reproduced the join timeout twice.
72+
- The fixed production build completed the exact heavy threaded-flush case in
73+
five runs, including three consecutive repetitions.
74+
- The normal threaded-flush, safepoint-handshake, and recorder-token gates were
75+
rerun after the fix.
76+
- A deterministic join reducer keeps one single-round background worker and
77+
repeats the existing hot churn/flush loop. The unfixed exact source failed at
78+
round 40 in three consecutive runs; the fixed build completed all 96 joins.
79+
- A production Lua regression enters both a timed channel receive and a timed
80+
join from a trace `start` callback. Both waits return their bounded timeout,
81+
preserve the active recorder frame, and allow the peer flusher to complete
82+
after callback unwind; this catches the unguarded teardown crash.
83+
- The focused join and VM-event park regressions also pass a clean
84+
`-DLUA_USE_ASSERT` build.

src/lib_threading.c

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -439,11 +439,22 @@ static int64_t threading_native_wait_slice(int has_timeout, int64_t ns)
439439
static uint32_t threading_futex_wait_l(lua_State *L, uint32_t *addr,
440440
uint32_t expect, int64_t ns)
441441
{
442+
global_State *g = G(L);
443+
uint32_t tid = lj_thr_current_id(g);
442444
uint32_t actions;
443445
/*
444446
** Public blocking waits are native parks, but the waiting TG must still be
445-
** visible to soft handshakes while parked.
447+
** visible to soft handshakes while parked. A C library call can be entered
448+
** while its owner still holds an asynchronously abortable recorder token.
449+
** Never park with that token: a peer which needs it may be the very thread
450+
** this wait is joining, creating an owner-token/join cycle with no runnable
451+
** side left to finish recorder cleanup.
446452
*/
453+
/* A VM-event callback is still executing inside trace_state() and therefore
454+
** cannot tear down J->cur reentrantly. Its bounded wait remains native, but
455+
** cleanup is deferred until that recorder frame unwinds. */
456+
if (tid == 0 || vmevent_owner_acq(g) != tid)
457+
lj_trace_abort_owner(L);
447458
lj_native_enter(L2TG(L));
448459
(void)la_futex_wait(addr, expect, ns);
449460
actions = lj_native_leave(L);

src/lj_chan.c

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "lj_safepoint.h"
1515
#include "lj_thr.h"
1616
#include "lj_tg.h"
17+
#include "lj_trace.h"
1718

1819
#include <errno.h>
1920
#include <limits.h>
@@ -112,9 +113,14 @@ static void chan_wait(lua_State *L, LJChan *ch)
112113
lj_safepoint_checkstop_fresh(L, 0, had_stopreq);
113114
return;
114115
}
115-
if (L)
116+
if (L) {
117+
global_State *g = G(L);
118+
uint32_t tid = lj_thr_current_id(g);
119+
/* Preserve an active VM-event recorder frame; see the timed twin below. */
120+
if (tid == 0 || vmevent_owner_acq(g) != tid)
121+
lj_trace_abort_owner(L);
116122
lj_native_enter_l(L, &frame); /* 09 section 9.5: channel park is native. */
117-
else if (tg)
123+
} else if (tg)
118124
lj_native_enter(tg);
119125
(void)la_futex_wait(&ch->futex, f, 1000000);
120126
if (L)
@@ -161,9 +167,16 @@ static int chan_wait_timeout(lua_State *L, LJChan *ch, int64_t ns)
161167
lj_safepoint_checkstop_fresh(L, 0, had_stopreq);
162168
return 0;
163169
}
164-
if (L)
170+
if (L) {
171+
global_State *g = G(L);
172+
uint32_t tid = lj_thr_current_id(g);
173+
/* trace_state() owns J->cur across VM-event callbacks. Never destroy that
174+
** state reentrantly; the callback's bounded park instead leaves cleanup to
175+
** the outer recorder unwind. Ordinary channel parks release ownership. */
176+
if (tid == 0 || vmevent_owner_acq(g) != tid)
177+
lj_trace_abort_owner(L);
165178
lj_native_enter_l(L, &frame); /* 09 section 9.5: timed channel park. */
166-
else if (tg)
179+
} else if (tg)
167180
lj_native_enter(tg);
168181
rc = la_futex_wait(&ch->futex, f, ns);
169182
if (L)

tests/suites/m6_jit.lua

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ local m6_cases = {
3939
"m6_jit_flush_gc_current_stack",
4040
"m6_jit_util_flush_race",
4141
"m6_jit_flush_thread_stress",
42+
"m6_jit_flush_join_token_liveness",
43+
"m6_jit_park_vmevent_reentrant",
4244
"m6_jit_flush_thread_heavy_stress",
4345
"m6_jit_mt_activation_flush",
4446
"m6_jit_gcworkers_activation_flush",
@@ -2229,6 +2231,39 @@ assert(live >= 4, live)
22292231
end
22302232
})
22312233

2234+
add({
2235+
name = "m6_jit_flush_join_token_liveness",
2236+
description = "blocking join releases asynchronously aborted recorder ownership",
2237+
run = function(t)
2238+
build_default(t)
2239+
-- This is the deterministic reducer for the heavy stress failure: the
2240+
-- top-level churn FORL owns the recorder token at round 40 while its
2241+
-- short-lived peer enters jit.flush() and must finish before join.
2242+
luajit_file(t, t:path("tests", "t-jit-flush-thread-stress.lua"), {
2243+
lua_path = true,
2244+
timeout = "20s",
2245+
env = {
2246+
LJ_M6_JIT_FLUSH_THREAD_THREADS = "1",
2247+
LJ_M6_JIT_FLUSH_THREAD_ROUNDS = "1",
2248+
LJ_M6_JIT_FLUSH_THREAD_CHURN = "96",
2249+
LJ_M6_JIT_FLUSH_THREAD_JOIN_TIMEOUT = "5"
2250+
}
2251+
})
2252+
print("M6 JIT blocking-join recorder-token liveness passed")
2253+
end
2254+
})
2255+
2256+
add({
2257+
name = "m6_jit_park_vmevent_reentrant",
2258+
description = "blocking parks preserve active VM-event recorder frames",
2259+
run = function(t)
2260+
build_default(t)
2261+
luajit_file(t, t:path("tests", "t-jit-park-vmevent-reentrant.lua"),
2262+
{ lua_path = true, timeout = "20s" })
2263+
print("M6 JIT VM-event park reentrancy passed")
2264+
end
2265+
})
2266+
22322267
add({
22332268
name = "m6_jit_flush_thread_heavy_stress",
22342269
description = "heavier threaded JIT flush stress with progress diagnostics",
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
local th = require"threading"
2+
3+
local function make_trace(n)
4+
local sum = 0
5+
for i = 1, n do sum = sum + i end
6+
return sum
7+
end
8+
9+
local function trigger_trace()
10+
for _ = 1, 40 do assert(make_trace(80) == 3240) end
11+
end
12+
13+
local function channel_park_from_trace_event()
14+
local entered = th.channel(1)
15+
local release = th.channel(1)
16+
local armed = true
17+
local saw_timeout = false
18+
19+
local function trace_hook(ev)
20+
if armed and ev == "start" then
21+
armed = false
22+
assert(entered:send("entered", 1) == true)
23+
local token, status = release:recv(0.05)
24+
saw_timeout = token == nil and status == "timeout"
25+
end
26+
end
27+
jit.off(trace_hook, true)
28+
29+
jit.flush()
30+
jit.opt.start("hotloop=1", "hotexit=1")
31+
jit.attach(trace_hook, "trace")
32+
local peer = th.spawn(function(entered_ch, release_ch)
33+
local token, ok = entered_ch:recv(1)
34+
assert(ok == true and token == "entered")
35+
jit.flush()
36+
assert(release_ch:send("release", 1) == true)
37+
return true
38+
end, entered, release)
39+
40+
trigger_trace()
41+
jit.attach(trace_hook)
42+
local joined, result = peer:join(5)
43+
assert(joined == true and result == true,
44+
"channel-event peer did not finish after recorder unwind")
45+
assert(saw_timeout,
46+
"channel event park unexpectedly crossed synchronous peer flush")
47+
end
48+
49+
local function join_park_from_trace_event()
50+
local entered = th.channel(1)
51+
local armed = true
52+
local saw_timeout = false
53+
local peer
54+
55+
local function trace_hook(ev)
56+
if armed and ev == "start" then
57+
armed = false
58+
assert(entered:send("entered", 1) == true)
59+
local joined, status = peer:join(0.05)
60+
saw_timeout = joined ~= true and status == "timeout"
61+
end
62+
end
63+
jit.off(trace_hook, true)
64+
65+
jit.flush()
66+
jit.opt.start("hotloop=1", "hotexit=1")
67+
jit.attach(trace_hook, "trace")
68+
peer = th.spawn(function(entered_ch)
69+
local token, ok = entered_ch:recv(1)
70+
assert(ok == true and token == "entered")
71+
jit.flush()
72+
return true
73+
end, entered)
74+
75+
trigger_trace()
76+
jit.attach(trace_hook)
77+
local joined, result = peer:join(5)
78+
assert(joined == true and result == true,
79+
"join-event peer did not finish after recorder unwind")
80+
assert(saw_timeout,
81+
"join event park unexpectedly crossed synchronous peer flush")
82+
end
83+
84+
channel_park_from_trace_event()
85+
join_park_from_trace_event()
86+
print("t-jit-park-vmevent-reentrant OK: parks defer owner teardown")

0 commit comments

Comments
 (0)