Skip to content

Commit e4fb703

Browse files
test: make the Init/Start race test detect the race (#24)
TestInitIsSafeWithConcurrentStart released Init and Start from a bare start gate. Measured against code with initMu removed, it reported the race 0 times in 10: Init takes runnerMu three times (destroyed, SetRuntimeContext, runnerEnv), and each acquisition orders the two bodies against Start's own runnerMu section, so the detector sees no concurrency. Its one postcondition — that a uvicorn process exists — also held in the interleaving where Start ran first and launched on port 0. #25 had already retired this shape for the teardown test; this repeats its fix, gating on the window Init is vulnerable in and repeating so the detection is reliable rather than sampled. The suite never exercised a cold Init, because a pre-installed environment sends Init down runnerEnv's already-published branch and skips CreateRunnerEnvironment — where the network mappings and the whole environment manager are read and ActiveEnv is published. That was untestable while #23 was open and is not any more, so the cold path gets its own overlap, with Start pointed at a missing source location so it reads everything and then fails in exec instead of launching a real uvicorn. Both tests now detect the race 5/5 with initMu removed. The 100ms sleep now fails fast: a Start that completes while Init is still gated is the defect, so it is observed and reported rather than waited out. It is deliberately only observed on that side — receiving from the channel on the passing path would order Start ahead of Init's release and hide the race. Reuses initTestRuntime rather than carrying a second copy of the same fixture, and stops indexing past --port blindly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent ea387a8 commit e4fb703

1 file changed

Lines changed: 84 additions & 69 deletions

File tree

‎init_start_test.go‎

Lines changed: 84 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -11,69 +11,29 @@ import (
1111

1212
basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0"
1313
runtimev0 "github.com/codefly-dev/core/generated/go/codefly/services/runtime/v0"
14-
"github.com/codefly-dev/core/resources"
15-
"github.com/codefly-dev/core/standards"
1614
"github.com/stretchr/testify/require"
1715
)
1816

19-
const initStartProject = `[project]
20-
name = "init-start-probe"
21-
version = "0.0.0"
22-
`
23-
24-
// initStartTestRuntime stands a Runtime up at the point Init is called from,
25-
// with a runner environment already in place so Init runs its publishing
26-
// section — runtime context, network mappings, ports, environment variables —
27-
// without building a real one.
17+
// initStartTestRuntime builds on initTestRuntime and adds what an Init/Start
18+
// overlap needs: a runner environment Start can launch against without a real
19+
// uv, a cache location for the dependency hashes, and a configuration, so Init
20+
// still has environment-manager work to do after its runner section — the
21+
// window a Start can land in.
2822
func initStartTestRuntime(t *testing.T) (*Runtime, *fakeRunnerEnvironment, *runtimev0.InitRequest) {
2923
t.Helper()
3024

31-
runtime := NewRuntime(NewService())
32-
runtime.Location = t.TempDir()
33-
// Load is what supplies these; these tests call Init and Start directly.
34-
runtime.Logger = runtime.Wool
35-
runtime.Identity = resources.ServiceIdentityFromProto(&basev0.ServiceIdentity{
36-
Name: "probe",
37-
Module: "mod",
38-
Workspace: "workspace",
39-
WorkspacePath: runtime.Location,
40-
})
41-
42-
source := t.TempDir()
43-
require.NoError(t, os.MkdirAll(filepath.Join(source, "src"), 0o700))
44-
require.NoError(t, os.WriteFile(filepath.Join(source, "pyproject.toml"), []byte(initStartProject), 0o600))
45-
require.NoError(t, os.WriteFile(filepath.Join(source, "src", "main.py"), []byte("app = None\n"), 0o600))
46-
runtime.Service.SourceLocation = source
25+
runtime, req := initTestRuntime(t)
4726
runtime.cacheLocation = t.TempDir()
48-
49-
endpoint := &basev0.Endpoint{Module: "mod", Service: "probe", Name: "rest", Api: standards.REST}
50-
runtime.FastAPI.RestEndpoint = endpoint
51-
5227
env := &fakeRunnerEnvironment{}
5328
runtime.runnerEnvironment = env
54-
t.Cleanup(runtime.Base.StopWatcher)
55-
56-
return runtime, env, &runtimev0.InitRequest{
57-
RuntimeContext: resources.NewRuntimeContextNative(),
58-
// A configuration leaves Init environment-manager work to do after its
59-
// runner section, which is where a Start can overlap it.
60-
Configuration: &basev0.Configuration{
61-
Origin: "mod/probe",
62-
Infos: []*basev0.ConfigurationInformation{{
63-
Name: "postgres",
64-
ConfigurationValues: []*basev0.ConfigurationValue{{Key: "url", Value: "postgres://localhost"}},
65-
}},
66-
},
67-
ProposedNetworkMappings: []*basev0.NetworkMapping{{
68-
Endpoint: endpoint,
69-
Instances: []*basev0.NetworkInstance{{
70-
Access: resources.NewNativeNetworkAccess(),
71-
Hostname: "localhost",
72-
Port: 9999,
73-
Address: "http://localhost:9999",
74-
}},
29+
req.Configuration = &basev0.Configuration{
30+
Origin: "mod/probe",
31+
Infos: []*basev0.ConfigurationInformation{{
32+
Name: "postgres",
33+
ConfigurationValues: []*basev0.ConfigurationValue{{Key: "url", Value: "postgres://localhost"}},
7534
}},
7635
}
36+
return runtime, env, req
7737
}
7838

7939
// uvicornProc returns the process Start launched, told apart from the ones Init
@@ -89,6 +49,16 @@ func uvicornProc(t *testing.T, env *fakeRunnerEnvironment) *fakeProc {
8949
return nil
9050
}
9151

52+
// uvicornPort returns the port Start bound the process to.
53+
func uvicornPort(t *testing.T, env *fakeRunnerEnvironment) string {
54+
t.Helper()
55+
args := uvicornProc(t, env).args
56+
i := slices.Index(args, "--port")
57+
require.GreaterOrEqual(t, i, 0, "uvicorn was launched without a port: %v", args)
58+
require.Less(t, i+1, len(args), "uvicorn was launched with a bare --port: %v", args)
59+
return args[i+1]
60+
}
61+
9262
// gatingEnv holds Init inside its runner section until the test releases it, so
9363
// a Start can be issued while Init is demonstrably mid-flight.
9464
type gatingEnv struct {
@@ -107,7 +77,19 @@ func (e *gatingEnv) Init(context.Context) error {
10777
// port Start binds uvicorn to, so a Start landing in the middle of an Init runs
10878
// after it and launches on the published port rather than on whatever the field
10979
// happened to hold.
80+
//
81+
// It is also where `-race` sees the two calls overlap on the state Init
82+
// publishes. Gating Init inside its runner section is what makes that reliable:
83+
// released from a bare start gate the two bodies order themselves through
84+
// runnerMu, and the detector reports nothing at all.
11085
func TestStartWaitsForInitToPublishItsPorts(t *testing.T) {
86+
for range 20 {
87+
runInitAgainstStart(t)
88+
}
89+
}
90+
91+
func runInitAgainstStart(t *testing.T) {
92+
t.Helper()
11193
runtime, fake, req := initStartTestRuntime(t)
11294
env := &gatingEnv{fakeRunnerEnvironment: fake, entered: make(chan struct{}), release: make(chan struct{})}
11395
runtime.runnerEnvironment = env
@@ -123,56 +105,89 @@ func TestStartWaitsForInitToPublishItsPorts(t *testing.T) {
123105

124106
<-env.entered
125107

108+
startDone := make(chan struct{})
126109
var startResp *runtimev0.StartResponse
127110
var startErr error
128111
wg.Add(1)
129112
go func() {
130113
defer wg.Done()
131114
startResp, startErr = runtime.Start(context.Background(), &runtimev0.StartRequest{})
115+
close(startDone)
132116
}()
133117

134118
// Nothing observable says the Start goroutine reached the lock, so give it
135-
// the room an unserialized Start would need to launch on the unwritten port.
136-
time.Sleep(100 * time.Millisecond)
119+
// the room an unserialized Start would need. Observing startDone is only
120+
// safe on the failing side: receiving from it would order the Start
121+
// goroutine ahead of the release below and hide the very race this test
122+
// exists to surface, so the good path must fall through the timeout having
123+
// synchronized with nothing.
124+
select {
125+
case <-startDone:
126+
t.Fatal("Start ran to completion while Init was still inside its runner section")
127+
case <-time.After(50 * time.Millisecond):
128+
}
137129
close(env.release)
138130
wg.Wait()
139131

140132
require.NoError(t, initErr)
141133
require.Equal(t, runtimev0.InitStatus_READY, initResp.GetStatus().GetState(), initResp.GetStatus().GetMessage())
142134
require.NoError(t, startErr)
143135
require.Equal(t, runtimev0.StartStatus_STARTED, startResp.GetStatus().GetState(), startResp.GetStatus().GetMessage())
136+
require.Equal(t, "9999", uvicornPort(t, fake))
137+
}
144138

145-
proc := uvicornProc(t, fake)
146-
require.Contains(t, proc.args, "--port")
147-
require.Equal(t, "9999", proc.args[slices.Index(proc.args, "--port")+1])
139+
// TestInitIsSafeWithConcurrentStartOnAColdRuntime covers the path the gated
140+
// test cannot reach: the first Init, which runs CreateRunnerEnvironment. That
141+
// function reads the network mappings and the whole environment manager, and
142+
// publishes ActiveEnv — none of it exercised once a runner environment is
143+
// already in place.
144+
//
145+
// Start is pointed at a source location that does not exist, so it does all of
146+
// its reading and then fails in exec rather than launching a real uvicorn.
147+
func TestInitIsSafeWithConcurrentStartOnAColdRuntime(t *testing.T) {
148+
isolateWorkingDir(t)
149+
for range 20 {
150+
runColdInitAgainstStart(t)
151+
}
148152
}
149153

150-
// TestInitIsSafeWithConcurrentStart overlaps the two calls that own the same
151-
// fields. Under `-race` this is what catches Init publishing the ports, the
152-
// network mappings, the runtime context and the environment-variable manager
153-
// outside the lock Start reads them under; without it, it still pins that
154-
// neither side panics however they interleave.
155-
func TestInitIsSafeWithConcurrentStart(t *testing.T) {
156-
runtime, env, req := initStartTestRuntime(t)
154+
func runColdInitAgainstStart(t *testing.T) {
155+
t.Helper()
156+
runtime, req := initTestRuntime(t)
157+
// Init creates and publishes the runner environment — the section under
158+
// test — and then stops on the missing project file, well before uv.
159+
require.NoError(t, os.Remove(filepath.Join(runtime.Service.SourceLocation, "pyproject.toml")))
160+
runtime.Service.SourceLocation = filepath.Join(runtime.Service.SourceLocation, "does-not-exist")
161+
162+
var initResp *runtimev0.InitResponse
163+
var initErr error
164+
var startResp *runtimev0.StartResponse
157165

158166
gate := make(chan struct{})
159167
var wg sync.WaitGroup
160-
var initErr, startErr error
161168
wg.Add(2)
162169
go func() {
163170
defer wg.Done()
164171
<-gate
165-
_, initErr = runtime.Init(context.Background(), req)
172+
initResp, initErr = runtime.Init(context.Background(), req)
166173
}()
167174
go func() {
168175
defer wg.Done()
169176
<-gate
170-
_, startErr = runtime.Start(context.Background(), &runtimev0.StartRequest{})
177+
// Land in the window Init is vulnerable in. Before the environment is
178+
// published Start refuses at once, having read none of the state Init
179+
// is writing, and the accesses that would race never execute.
180+
waitForRunnerEnvironment(runtime)
181+
startResp, _ = runtime.Start(context.Background(), &runtimev0.StartRequest{})
171182
}()
172183
close(gate)
173184
wg.Wait()
174185

175186
require.NoError(t, initErr)
176-
require.NoError(t, startErr)
177-
uvicornProc(t, env)
187+
require.NotNil(t, initResp, "Init must not panic when a Start lands mid-flight")
188+
require.Contains(t, initResp.GetStatus().GetMessage(), "no pyproject.toml",
189+
"Init must run past its runner section, or this tests nothing")
190+
require.NotNil(t, startResp, "Start must not panic against a concurrent Init")
191+
require.DirExists(t, filepath.Join(runtime.Location, ".cache/local"),
192+
"Init must have created and published the runner environment")
178193
}

0 commit comments

Comments
 (0)