Skip to content

Commit edce1cd

Browse files
fix(rerank): prevent Darwin traced-exec deadlock
1 parent cd890a7 commit edce1cd

2 files changed

Lines changed: 84 additions & 2 deletions

File tree

‎pkg/rerank/server_test.go‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,46 @@ func TestMain(m *testing.M) {
5656
os.Exit(m.Run())
5757
}
5858

59+
func TestRunnerDisablesAsyncPreemptionOnlyBeforeTargetExec(t *testing.T) {
60+
find := func(environment []string, name string) (string, bool) {
61+
prefix := name + "="
62+
for _, entry := range environment {
63+
if strings.HasPrefix(entry, prefix) {
64+
return strings.TrimPrefix(entry, prefix), true
65+
}
66+
}
67+
return "", false
68+
}
69+
tests := []struct {
70+
name string
71+
environment []string
72+
wantDebug string
73+
wantDebugSet bool
74+
}{
75+
{name: "unset"},
76+
{name: "empty", environment: []string{"GODEBUG="}, wantDebugSet: true},
77+
{name: "preserved", environment: []string{"GODEBUG=cgocheck=0,asyncpreemptoff=0"}, wantDebug: "cgocheck=0,asyncpreemptoff=0", wantDebugSet: true},
78+
}
79+
for _, test := range tests {
80+
t.Run(test.name, func(t *testing.T) {
81+
runner := environmentForRunner(test.environment, "test-instance")
82+
runnerDebug, ok := find(runner, "GODEBUG")
83+
if !ok || !strings.HasSuffix(runnerDebug, "asyncpreemptoff=1") {
84+
t.Fatalf("runner GODEBUG = %q, %v; want asyncpreemptoff=1", runnerDebug, ok)
85+
}
86+
87+
target := environmentForConstrainedReranker(runner)
88+
targetDebug, ok := find(target, "GODEBUG")
89+
if targetDebug != test.wantDebug || ok != test.wantDebugSet {
90+
t.Fatalf("target GODEBUG = %q, %v; want %q, %v", targetDebug, ok, test.wantDebug, test.wantDebugSet)
91+
}
92+
if _, ok := find(target, supervisorRunnerDebugEnv); ok {
93+
t.Fatal("target retained private runner GODEBUG state")
94+
}
95+
})
96+
}
97+
}
98+
5999
func TestRerankerManagerStopRefusesUnownedLivePID(t *testing.T) {
60100
dir := t.TempDir()
61101
pid := os.Getpid()
@@ -120,7 +160,8 @@ func TestRerankerManagerStartWorksForEmbeddedHost(t *testing.T) {
120160
}
121161
t.Cleanup(func() { _ = mgr.Stop() })
122162
if err := mgr.Start(); err != nil {
123-
t.Fatal(err)
163+
log, _ := os.ReadFile(filepath.Join(home, "reranker.log"))
164+
t.Fatalf("%v\nreranker log:\n%s", err, log)
124165
}
125166
state, err := mgr.readPIDState()
126167
if err != nil {

‎pkg/rerank/supervisor.go‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ const (
3737
supervisorHomeEnv = "SGREP_RERANKER_SUPERVISOR_HOME"
3838
supervisorPortEnv = "SGREP_RERANKER_SUPERVISOR_PORT"
3939
supervisorLifecycleEnv = "SGREP_RERANKER_SUPERVISOR_LIFECYCLE_LOCK"
40+
supervisorRunnerDebugEnv = "SGREP_RERANKER_SUPERVISOR_RUNNER_GODEBUG"
4041
supervisorProtocolVersion = 1
4142
guardianCleanShutdown = byte(0x73)
4243
guardianReady = byte(0x61)
@@ -320,7 +321,7 @@ func runConstrainedReranker(args []string) error {
320321
if command[0] != runnerContinue {
321322
return fmt.Errorf("invalid constrained reranker authorization")
322323
}
323-
environment := withoutSupervisorEnvironment(os.Environ())
324+
environment := environmentForConstrainedReranker(os.Environ())
324325
if err := syscall.Exec(executable, commandArgs, environment); err != nil {
325326
return fmt.Errorf("exec constrained reranker: %w", err)
326327
}
@@ -1318,12 +1319,52 @@ func environmentForGuardian(environment []string) []string {
13181319

13191320
func environmentForRunner(environment []string, instance string) []string {
13201321
result := environmentForReranker(environment, instance)
1322+
filtered := result[:0]
1323+
runnerDebug := "asyncpreemptoff=1"
1324+
for _, entry := range result {
1325+
if strings.HasPrefix(entry, "GODEBUG=") {
1326+
original := strings.TrimPrefix(entry, "GODEBUG=")
1327+
filtered = append(filtered, supervisorRunnerDebugEnv+"="+original)
1328+
if original != "" {
1329+
runnerDebug = original + "," + runnerDebug
1330+
}
1331+
continue
1332+
}
1333+
filtered = append(filtered, entry)
1334+
}
1335+
// Darwin's PT_TRACE_ME can delay an asynchronous preemption signal forever,
1336+
// deadlocking runtime_BeforeExec. The setting is removed before the target
1337+
// exec, so it applies only to the short-lived Go runner.
1338+
result = append(filtered, "GODEBUG="+runnerDebug)
13211339
return append(result,
13221340
supervisorModeEnv+"=1",
13231341
supervisorProtocolEnv+"="+strconv.Itoa(supervisorProtocolVersion),
13241342
)
13251343
}
13261344

1345+
func environmentForConstrainedReranker(environment []string) []string {
1346+
var originalDebug string
1347+
var restoreDebug bool
1348+
debugPrefix := supervisorRunnerDebugEnv + "="
1349+
for _, entry := range environment {
1350+
if strings.HasPrefix(entry, debugPrefix) {
1351+
originalDebug = strings.TrimPrefix(entry, debugPrefix)
1352+
restoreDebug = true
1353+
}
1354+
}
1355+
result := withoutSupervisorEnvironment(environment)
1356+
filtered := result[:0]
1357+
for _, entry := range result {
1358+
if !strings.HasPrefix(entry, "GODEBUG=") {
1359+
filtered = append(filtered, entry)
1360+
}
1361+
}
1362+
if restoreDebug {
1363+
filtered = append(filtered, "GODEBUG="+originalDebug)
1364+
}
1365+
return filtered
1366+
}
1367+
13271368
func environmentForReranker(environment []string, instance string) []string {
13281369
result := withoutSupervisorEnvironment(environment)
13291370
prefix := rerankerInstanceEnv + "="

0 commit comments

Comments
 (0)