fix(sandbox): redefine deletion semantics around VMM stop and resource cleanup-Conch-AtomGit

Mdocs/user/python-sdk.md+2-0

	@@ -167,6 +167,8 @@ sandbox.delete(sandbox_id=None) -> bool
167		167
168	成功删除当前实例对应的沙箱后，SDK 清空该对象缓存的身份、元数据和运行时信息，并关闭 Agent 客户端。删除失败或删除其他 ID 的沙箱时，不修改当前对象。	168	成功删除当前实例对应的沙箱后，SDK 清空该对象缓存的身份、元数据和运行时信息，并关闭 Agent 客户端。删除失败或删除其他 ID 的沙箱时，不修改当前对象。
169		169
		170	+删除以 VMM 停止成功为提交点：VMM 停止失败时保留 Sandbox 记录和运行资源并返回错误；VMM 已停止后，即使网络、Volume、启动目录或 CID 等后续资源清理失败，Conch 也会记录错误、移除 Sandbox 记录并返回成功。清理失败的资源不会重新进入可复用资源池。
		171	+
170	静态方法：	172	静态方法：
171	```text	173	```text
172	Sandbox.delete_sandbox(sandbox_id) -> bool	174	Sandbox.delete_sandbox(sandbox_id) -> bool

Minternal/conchruntime/service.go+2-5

	@@ -416,10 +416,7 @@ func (s *Service) RemoveSandbox(ctx context.Context, sandboxID string) error {
416	}	416	}
417	}	417	}
418	cleanupErr := s.Sandbox.Delete(sandbox.DeleteRequest{SandboxID: sandboxID})	418	cleanupErr := s.Sandbox.Delete(sandbox.DeleteRequest{SandboxID: sandboxID})
419	- if errors.Is(cleanupErr, sandbox.ErrNotFound) {	419	+ if cleanupErr != nil && !errors.Is(cleanupErr, sandbox.ErrNotFound) {
420	- cleanupErr = nil
421	- }
422	- if errors.Is(cleanupErr, sandbox.ErrFailedPrecondition) {
423	return cleanupErr	420	return cleanupErr
424	}	421	}
425	var deleteErr error	422	var deleteErr error
	@@ -429,7 +426,7 @@ func (s *Service) RemoveSandbox(ctx context.Context, sandboxID string) error {
429	if deleteErr == nil && rec.ID != "" {	426	if deleteErr == nil && rec.ID != "" {
430	s.publishLifecycleEvent(webhook.EventSandboxKilled, rec, "request")	427	s.publishLifecycleEvent(webhook.EventSandboxKilled, rec, "request")
431	}	428	}
432	- return combineOperationErrors(cleanupErr, deleteErr)	429	+ return deleteErr
		J J jing-ruiSep 4 这里为什么要丢掉 cleanupErr? Y ytluo Sep 4 Comments： Sep 4 Comments：
433	}	430	}
434		431
435	// HandleSandboxUnexpectedExit records the loss of a sandbox and emits its lifecycle event.	432	// HandleSandboxUnexpectedExit records the loss of a sandbox and emits its lifecycle event.

Minternal/conchruntime/service_test.go+4-28

	@@ -615,24 +615,9 @@ func TestCheckpointSandboxBuildsConsecutiveTemplateLineage(t *testing.T) {
615	}	615	}
616	}	616	}
617		617
618	-func TestRemoveSandboxDeletesRecordWhenCleanupFails(t *testing.T) {	618	+func TestRemoveSandboxKeepsRecordWhenVMMStopFails(t *testing.T) {
619	ctx := context.Background()	619	ctx := context.Background()
620	store := newTestStore(t)	620	store := newTestStore(t)
621	- events := make(chan webhook.Event, 1)
622	- receiver := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
623	- var event webhook.Event
624	- if err := json.NewDecoder(r.Body).Decode(&event); err != nil {
625	- t.Errorf("decode event: %v", err)
626	- return
627	- }
628	- events <- event
629	- }))
630	- defer receiver.Close()
631	- dispatcher := webhook.NewDispatcher()
632	- if _, err := dispatcher.Create(runtimeapi.WebhookCreateOptions{Name: "receiver", URL: receiver.URL}); err != nil {
633	- t.Fatalf("register webhook: %v", err)
634	- }
635	-
636	record := sandbox.Record{	621	record := sandbox.Record{
637	ID: "sandbox-1",	622	ID: "sandbox-1",
638	State: sandbox.StateReady,	623	State: sandbox.StateReady,
	@@ -642,23 +627,14 @@ func TestRemoveSandboxDeletesRecordWhenCleanupFails(t *testing.T) {
642	t.Fatalf("UpsertSandbox() error = %v", err)	627	t.Fatalf("UpsertSandbox() error = %v", err)
643	}	628	}
644		629
645	- wantErr := errors.New("cleanup failed")	630	+ wantErr := errors.New("VMM stop failed")
646	sandboxOps := &fakeSandboxOps{deleteErr: wantErr}	631	sandboxOps := &fakeSandboxOps{deleteErr: wantErr}
647	svc := New(sandboxOps, nil, store)	632	svc := New(sandboxOps, nil, store)
648	- svc.WebhookDispatcher = dispatcher
649	if err := svc.RemoveSandbox(ctx, record.ID); !errors.Is(err, wantErr) {	633	if err := svc.RemoveSandbox(ctx, record.ID); !errors.Is(err, wantErr) {
650	t.Fatalf("RemoveSandbox() error = %v, want %v", err, wantErr)	634	t.Fatalf("RemoveSandbox() error = %v, want %v", err, wantErr)
651	}	635	}
652	- if _, err := store.Get(ctx, record.ID); !errors.Is(err, sandbox.ErrNotFound) {	636	+ if _, err := store.Get(ctx, record.ID); err != nil {
653	- t.Fatalf("GetSandbox() error = %v, want ErrNotFound", err)	637	+ t.Fatalf("GetSandbox() error = %v, want record retained", err)
654	- }
655	- select {
656	- case event := <-events:
657	- if event.Type != webhook.EventSandboxKilled \|\| event.EventData.KillReason != "request" \|\| event.SandboxID != record.ID {
658	- t.Fatalf("killed event = %#v", event)
659	- }
660	- case <-time.After(time.Second):
661	- t.Fatal("killed event not delivered after record deletion")
662	}	638	}
663	}	639	}
664		640

Minternal/sandbox/manager.go+59-19

	@@ -428,7 +428,10 @@ func (m *Manager) Create(parent context.Context, req CreateRequest) (result Crea
428	vmStartSpec.VirtioFS = volumeDevicesToDriver(volumeDevices)	428	vmStartSpec.VirtioFS = volumeDevicesToDriver(volumeDevices)
429	sbx, err := m.startSandbox(ctx, req, vmStartSpec, runtimeIDs, boot.Runtime.Resume)	429	sbx, err := m.startSandbox(ctx, req, vmStartSpec, runtimeIDs, boot.Runtime.Resume)
430	if err != nil {	430	if err != nil {
431	- m.cleanupCreateFailure(sbx, req.SandboxID)	431	+ cleanupErr := m.cleanupCreateFailure(sbx, req.SandboxID)
		432	+ if cleanupErr != nil {
		433	+ err = errors.Join(err, cleanupErr)
		434	+ }
432	switch {	435	switch {
433	case errors.Is(err, agentprotocol.ErrInvalidEnvironment):	436	case errors.Is(err, agentprotocol.ErrInvalidEnvironment):
434	return CreateResult{}, ErrInvalidEnvironment.Wrap(err)	437	return CreateResult{}, ErrInvalidEnvironment.Wrap(err)
	@@ -541,16 +544,26 @@ func (m *Manager) startSandbox(ctx context.Context, req CreateRequest, vmStartSp
541	)	544	)
542	}	545	}
543		546
544	-func (m Manager) cleanupCreateFailure(sbx Sandbox, sandboxID string) {	547	+func (m Manager) cleanupCreateFailure(sbx Sandbox, sandboxID string) error {
545	logger := ulog.GetLogger()	548	logger := ulog.GetLogger()
546	- if sbx != nil {	549	+ if sbx == nil {
547	- if closeErr := sbx.Close(context.Background()); closeErr != nil {	550	+ return nil
548	- logger.Warn("failed to cleanup sandbox after create failure",
549	- ulog.F("sandbox_id", sandboxID),
550	- ulog.F("error", closeErr),
551	- )
552	- }
553	}	551	}
		552	+ if stopErr := sbx.Stop(context.Background()); stopErr != nil {
		553	+ logger.Warn("failed to stop sandbox after create failure",
		554	+ ulog.F("sandbox_id", sandboxID),
		555	+ ulog.F("error", stopErr),
		556	+ )
		557	+ return stopErr
		558	+ }
		559	+ if cleanupErr := sbx.CleanupResources(context.Background()); cleanupErr != nil {
		560	+ logger.Warn("failed to cleanup sandbox after create failure",
		561	+ ulog.F("sandbox_id", sandboxID),
		562	+ ulog.F("error", cleanupErr),
		563	+ )
		564	+ return cleanupErr
		565	+ }
		566	+ return nil
554	}	567	}
555		568
556	func (m Manager) trackSandbox(ctx context.Context, mapKey string, entry sandboxEntry, sandboxID string, sbx *Sandbox, virtiofsExit <-chan struct{}) {	569	func (m Manager) trackSandbox(ctx context.Context, mapKey string, entry sandboxEntry, sandboxID string, sbx *Sandbox, virtiofsExit <-chan struct{}) {
	@@ -565,22 +578,39 @@ func (m Manager) trackSandbox(ctx context.Context, mapKey string, entry sandbo
565	close(vmmExit)	578	close(vmmExit)
566	}()	579	}()
567		580
		581	+ vmmStopped := false
568	select {	582	select {
569	case <-vmmExit:	583	case <-vmmExit:
		584	+ vmmStopped = true
570	case <-virtiofsExit:	585	case <-virtiofsExit:
571	}	586	}
572	- m.handleSandboxExit(mapKey, entry, sandboxID, sbx)	587	+ if err := m.handleSandboxExit(mapKey, entry, sandboxID, sbx, vmmStopped); err != nil {
		588	+ // Stop was not confirmed. Keep monitoring without holding entry.mu;
		589	+ // a concurrent Delete may finish cleanup before the VMM exits.
		590	+ <-vmmExit
		591	+ m.handleSandboxExit(mapKey, entry, sandboxID, sbx, true)
		592	+ }
573	}()	593	}()
574	}	594	}
575		595
576	-func (m Manager) handleSandboxExit(mapKey string, entry sandboxEntry, sandboxID string, sbx *Sandbox) {	596	+func (m Manager) handleSandboxExit(mapKey string, entry sandboxEntry, sandboxID string, sbx *Sandbox, vmmStopped bool) error {
577	logger := ulog.GetLogger()	597	logger := ulog.GetLogger()
578	entry.mu.Lock()	598	entry.mu.Lock()
579	if !m.isCurrentSandboxEntry(mapKey, entry) \|\| entry.sbx != sbx {	599	if !m.isCurrentSandboxEntry(mapKey, entry) \|\| entry.sbx != sbx {
580	entry.mu.Unlock()	600	entry.mu.Unlock()
581	- return	601	+ return nil
582	}	602	}
583	- cleanupErr := m.cleanupSandbox(context.Background(), sbx, sandboxID)	603	+ if !vmmStopped {
		604	+ if err := sbx.Stop(context.Background()); err != nil {
		605	+ entry.mu.Unlock()
		606	+ logger.Warn("failed to stop VMM after sandbox dependency exited",
		607	+ ulog.F("sandbox_id", sandboxID),
		608	+ ulog.F("error", err),
		609	+ )
		610	+ return err
		611	+ }
		612	+ }
		613	+ cleanupErr := m.cleanupSandboxResources(context.Background(), sbx, sandboxID)
584	if cleanupErr != nil {	614	if cleanupErr != nil {
585	logger.Warn("failed to cleanup sandbox after wait", ulog.F("sandbox_id", sandboxID), ulog.F("error", cleanupErr))	615	logger.Warn("failed to cleanup sandbox after wait", ulog.F("sandbox_id", sandboxID), ulog.F("error", cleanupErr))
586	}	616	}
	@@ -589,6 +619,7 @@ func (m Manager) handleSandboxExit(mapKey string, entry sandboxEntry, sandboxI
589	if m.UnexpectedExitHandler != nil {	619	if m.UnexpectedExitHandler != nil {
590	m.UnexpectedExitHandler(sandboxID, cleanupErr)	620	m.UnexpectedExitHandler(sandboxID, cleanupErr)
591	}	621	}
		622	+ return nil
592	}	623	}
593		624
594	func buildSandboxCreateResult(req CreateRequest, sbx *Sandbox, boot PreparedBoot, runtimeIDs createRuntimeIDs, volumeDevices []volume.Device) CreateResult {	625	func buildSandboxCreateResult(req CreateRequest, sbx *Sandbox, boot PreparedBoot, runtimeIDs createRuntimeIDs, volumeDevices []volume.Device) CreateResult {
	@@ -598,7 +629,7 @@ func buildSandboxCreateResult(req CreateRequest, sbx *Sandbox, boot PreparedBoot
598	AgentToken: req.AgentToken,	629	AgentToken: req.AgentToken,
599	SandboxID: req.SandboxID,	630	SandboxID: req.SandboxID,
600	VMMPID: sbx.process.Pid(),	631	VMMPID: sbx.process.Pid(),
601	- VMMSocketPath: sbx.process.VmmSocketPath,	632	+ VMMSocketPath: sbx.process.SocketPath(),
602	VsockCID: runtimeIDs.vsockCID,	633	VsockCID: runtimeIDs.vsockCID,
603	VsockSocketPath: runtimeIDs.vsockSocketPath,	634	VsockSocketPath: runtimeIDs.vsockSocketPath,
604	NetworkSlotID: sbx.slot.ID(),	635	NetworkSlotID: sbx.slot.ID(),
	@@ -627,15 +658,15 @@ func registerSandboxVolumeCleanup(sb Sandbox, volumeManager volume.Manager, sa
627	})	658	})
628	}	659	}
629		660
630	-func (m Manager) cleanupSandbox(ctx context.Context, sbx Sandbox, sandboxID string) error {	661	+func (m Manager) cleanupSandboxResources(ctx context.Context, sbx Sandbox, sandboxID string) error {
631	logger := ulog.GetLogger()	662	logger := ulog.GetLogger()
632	var errs []error	663	var errs []error
633	fields := []ulog.Field{	664	fields := []ulog.Field{
634	ulog.F("sandbox_id", sandboxID),	665	ulog.F("sandbox_id", sandboxID),
635	}	666	}
636		667
637	- finishClose := cleanupdiag.Start("sandbox.close", fields...)	668	+ finishClose := cleanupdiag.Start("sandbox.resources.release", fields...)
638	- err := sbx.Close(ctx)	669	+ err := sbx.CleanupResources(ctx)
639	finishClose(err)	670	finishClose(err)
640	if err != nil {	671	if err != nil {
641	logger.Warn("sandbox runtime cleanup is incomplete",	672	logger.Warn("sandbox runtime cleanup is incomplete",
	@@ -689,9 +720,18 @@ func (m *Manager) Delete(req DeleteRequest) error {
689	return fmt.Errorf("invalid sandbox entry for %s: sandbox is nil", req.SandboxID)	720	return fmt.Errorf("invalid sandbox entry for %s: sandbox is nil", req.SandboxID)
690	}	721	}
691		722
692	- err = m.cleanupSandbox(context.Background(), sbx, req.SandboxID)	723	+ if err := sbx.Stop(context.Background()); err != nil {
		724	+ return err
		725	+ }
		726	+ cleanupErr := m.cleanupSandboxResources(context.Background(), sbx, req.SandboxID)
		727	+ if cleanupErr != nil {
		728	+ ulog.GetLogger().Warn("sandbox stopped but resource cleanup was incomplete",
		729	+ ulog.F("sandbox_id", req.SandboxID),
		730	+ ulog.F("error", cleanupErr),
		731	+ )
		732	+ }
693	m.sandboxes.CompareAndDelete(mapKey, entry)	733	m.sandboxes.CompareAndDelete(mapKey, entry)
694	- return err	734	+ return nil
695	}	735	}
696		736
697	func (m *Manager) Suspend(req LifecycleRequest) error {	737	func (m *Manager) Suspend(req LifecycleRequest) error {

Minternal/sandbox/manager_test.go+131-13

	@@ -69,7 +69,7 @@ func TestHandleSandboxExitCleansSuspendedSandbox(t *testing.T) {
69	mapKey := "sandbox-a"	69	mapKey := "sandbox-a"
70	m.sandboxes.Store(mapKey, entry)	70	m.sandboxes.Store(mapKey, entry)
71		71
72	- m.handleSandboxExit(mapKey, entry, "sandbox-a", sbx)	72	+ m.handleSandboxExit(mapKey, entry, "sandbox-a", sbx, true)
73		73
74	if _, ok := m.sandboxes.Load(mapKey); ok {	74	if _, ok := m.sandboxes.Load(mapKey); ok {
75	t.Fatal("suspended sandbox entry remains after VMM exit")	75	t.Fatal("suspended sandbox entry remains after VMM exit")
	@@ -80,15 +80,18 @@ func TestHandleSandboxExitCleansSuspendedSandbox(t *testing.T) {
80	}	80	}
81		81
82	func TestHandleSandboxExitCallsUnexpectedHandlerOnce(t *testing.T) {	82	func TestHandleSandboxExitCallsUnexpectedHandlerOnce(t *testing.T) {
83	- m, entry, sbx := newExitTestSandbox(func(context.Context) error { return nil })	83	+ m := &Manager{boot: &recordingBootPreparer{}, cidAllocator: NewCIDAllocator()}
		84	+ sbx := &Sandbox{cleanup: NewCleanup(), sandboxID: "sandbox-a"}
		85	+ entry := &sandboxEntry{state: sandboxReady, sbx: sbx}
		86	+ m.sandboxes.Store("sandbox-a", entry)
84	type exitResult struct {	87	type exitResult struct {
85	id string	88	id string
86	err error	89	err error
87	}	90	}
88	called := make(chan exitResult, 2)	91	called := make(chan exitResult, 2)
89	m.UnexpectedExitHandler = func(id string, err error) { called <- exitResult{id: id, err: err} }	92	m.UnexpectedExitHandler = func(id string, err error) { called <- exitResult{id: id, err: err} }
90	- m.handleSandboxExit("sandbox-a", entry, "sandbox-a", sbx)	93	+ m.handleSandboxExit("sandbox-a", entry, "sandbox-a", sbx, true)
91	- m.handleSandboxExit("sandbox-a", entry, "sandbox-a", sbx)	94	+ m.handleSandboxExit("sandbox-a", entry, "sandbox-a", sbx, true)
92	select {	95	select {
93	case result := <-called:	96	case result := <-called:
94	if result.id != "sandbox-a" \|\| result.err != nil {	97	if result.id != "sandbox-a" \|\| result.err != nil {
	@@ -215,18 +218,106 @@ func TestCreateUsesLiveBoundedContextForBootCleanupAfterCancellation(t *testing.
215	}	218	}
216	}	219	}
217		220
		221	+type fakeSandboxProcess struct {
		222	+ stopErr error
		223	+}
		224	+
		225	+func (p *fakeSandboxProcess) Stop() error { return p.stopErr }
		226	+func (*fakeSandboxProcess) Wait() error { return nil }
		227	+func (*fakeSandboxProcess) Pause(context.Context) error { return nil }
		228	+func (*fakeSandboxProcess) ResumeVM(context.Context) error { return nil }
		229	+func (*fakeSandboxProcess) CreateSnapshot(context.Context, string) error { return nil }
		230	+func (*fakeSandboxProcess) Pid() int { return 0 }
		231	+func (*fakeSandboxProcess) SocketPath() string { return "" }
		232	+
		233	+type delayedExitProcess struct {
		234	+ fakeSandboxProcess
		235	+ exited chan struct{}
		236	+ stopAttempt chan struct{}
		237	+}
		238	+
		239	+func (p *delayedExitProcess) Wait() error {
		240	+ <-p.exited
		241	+ return nil
		242	+}
		243	+
		244	+func (p *delayedExitProcess) Stop() error {
		245	+ close(p.stopAttempt)
		246	+ return p.stopErr
		247	+}
		248	+
		249	+func TestTrackSandboxCleansAfterFailedStopAndLaterExit(t *testing.T) {
		250	+ cleanupCalls := 0
		251	+ m, entry, sbx := newExitTestSandbox(func(context.Context) error {
		252	+ cleanupCalls++
		253	+ return nil
		254	+ })
		255	+ process := &delayedExitProcess{
		256	+ fakeSandboxProcess: fakeSandboxProcess{stopErr: errors.New("stop failed")},
		257	+ exited: make(chan struct{}), stopAttempt: make(chan struct{}),
		258	+ }
		259	+ sbx.process = process
		260	+ exitReported := make(chan struct{}, 2)
		261	+ m.UnexpectedExitHandler = func(string, error) { exitReported <- struct{}{} }
		262	+ virtiofsExit := make(chan struct{})
		263	+ m.trackSandbox(context.Background(), "sandbox-a", entry, "sandbox-a", sbx, virtiofsExit)
		264	+ defer func() {
		265	+ select {
		266	+ case <-process.exited:
		267	+ default:
		268	+ close(process.exited)
		269	+ }
		270	+ }()
		271	+ close(virtiofsExit)
		272	+ select {
		273	+ case <-process.stopAttempt:
		274	+ case <-time.After(time.Second):
		275	+ t.Fatal("dependency exit did not trigger Stop")
		276	+ }
		277	+ entry.mu.Lock()
		278	+ retained := m.isCurrentSandboxEntry("sandbox-a", entry)
		279	+ cleanedBeforeExit := cleanupCalls
		280	+ entry.mu.Unlock()
		281	+ if !retained \|\| cleanedBeforeExit != 0 {
		282	+ t.Fatal("stop failure released sandbox ownership or resources")
		283	+ }
		284	+ select {
		285	+ case <-exitReported:
		286	+ t.Fatal("exit reported before confirmed VMM exit")
		287	+ default:
		288	+ }
		289	+ close(process.exited)
		290	+ select {
		291	+ case <-exitReported:
		292	+ case <-time.After(time.Second):
		293	+ t.Fatal("later VMM exit did not complete cleanup and notification")
		294	+ }
		295	+ if cleanupCalls != 1 \|\| m.isCurrentSandboxEntry("sandbox-a", entry) {
		296	+ t.Fatalf("cleanup calls = %d, want one cleanup and removed entry", cleanupCalls)
		297	+ }
		298	+ m.handleSandboxExit("sandbox-a", entry, "sandbox-a", sbx, true)
		299	+ select {
		300	+ case <-exitReported:
		301	+ t.Fatal("duplicate exit notification")
		302	+ default:
		303	+ }
		304	+}
		305	+
218	func TestDeleteRemovesEntryAfterCleanupFailure(t *testing.T) {	306	func TestDeleteRemovesEntryAfterCleanupFailure(t *testing.T) {
219	wantErr := errors.New("runtime cleanup failed")	307	wantErr := errors.New("runtime cleanup failed")
220	boot := &recordingBootPreparer{}	308	boot := &recordingBootPreparer{}
221	- m := &Manager{boot: boot, cidAllocator: NewCIDAllocator()}	309	+ m := &Manager{
		310	+ boot: boot,
		311	+ cidAllocator: NewCIDAllocator(),
		312	+ }
222	cleanup := NewCleanup()	313	cleanup := NewCleanup()
223	cleanup.Add(func(context.Context) error { return wantErr })	314	cleanup.Add(func(context.Context) error { return wantErr })
224	- sbx := &Sandbox{cleanup: cleanup, sandboxID: "sandbox-a"}	315	+ sbx := &Sandbox{process: &fakeSandboxProcess{}, cleanup: cleanup, sandboxID: "sandbox-a"}
225	entry := &sandboxEntry{state: sandboxReady, sbx: sbx}	316	entry := &sandboxEntry{state: sandboxReady, sbx: sbx}
226	m.sandboxes.Store("sandbox-a", entry)	317	m.sandboxes.Store("sandbox-a", entry)
227		318
228	- if err := m.Delete(DeleteRequest{SandboxID: "sandbox-a"}); !errors.Is(err, wantErr) {	319	+ if err := m.Delete(DeleteRequest{SandboxID: "sandbox-a"}); err != nil {
229	- t.Fatalf("Delete() error = %v, want %v", err, wantErr)	320	+ t.Fatalf("Delete() error = %v", err)
230	}	321	}
231	if _, ok := m.sandboxes.Load("sandbox-a"); ok {	322	if _, ok := m.sandboxes.Load("sandbox-a"); ok {
232	t.Fatal("sandbox entry remains after one-shot cleanup failure")	323	t.Fatal("sandbox entry remains after one-shot cleanup failure")
	@@ -243,7 +334,7 @@ func TestUnexpectedExitRemovesEntryAndReportsCleanupError(t *testing.T) {
243	called := make(chan error, 1)	334	called := make(chan error, 1)
244	m.UnexpectedExitHandler = func(_ string, err error) { called <- err }	335	m.UnexpectedExitHandler = func(_ string, err error) { called <- err }
245		336
246	- m.handleSandboxExit("sandbox-a", entry, "sandbox-a", sbx)	337	+ m.handleSandboxExit("sandbox-a", entry, "sandbox-a", sbx, true)
247	if _, ok := m.sandboxes.Load("sandbox-a"); ok {	338	if _, ok := m.sandboxes.Load("sandbox-a"); ok {
248	t.Fatal("sandbox entry remains after one-shot unexpected-exit cleanup")	339	t.Fatal("sandbox entry remains after one-shot unexpected-exit cleanup")
249	}	340	}
	@@ -271,7 +362,7 @@ func TestWaitForSandboxExitCleansSandboxOnVirtiofsExit(t *testing.T) {
271	case <-vmmExit:	362	case <-vmmExit:
272	case <-virtiofsExit:	363	case <-virtiofsExit:
273	}	364	}
274	- m.handleSandboxExit("sandbox-a", entry, "sandbox-a", sbx)	365	+ m.handleSandboxExit("sandbox-a", entry, "sandbox-a", sbx, false)
275	}()	366	}()
276	close(virtiofsExit)	367	close(virtiofsExit)
277		368
	@@ -300,12 +391,14 @@ func TestWaitForSandboxExitDoesNotDuplicateDeleteCleanup(t *testing.T) {
300		391
301	vmmExit := make(chan struct{})	392	vmmExit := make(chan struct{})
302	virtiofsExit := make(chan struct{})	393	virtiofsExit := make(chan struct{})
		394	+ exitHandled := make(chan struct{})
303	go func() {	395	go func() {
304	select {	396	select {
305	case <-vmmExit:	397	case <-vmmExit:
306	case <-virtiofsExit:	398	case <-virtiofsExit:
307	}	399	}
308	- m.handleSandboxExit("sandbox-a", entry, "sandbox-a", sbx)	400	+ m.handleSandboxExit("sandbox-a", entry, "sandbox-a", sbx, false)
		401	+ close(exitHandled)
309	}()	402	}()
310	deleteDone := make(chan error, 1)	403	deleteDone := make(chan error, 1)
311	go func() {	404	go func() {
	@@ -328,6 +421,11 @@ func TestWaitForSandboxExitDoesNotDuplicateDeleteCleanup(t *testing.T) {
328	case <-time.After(time.Second):	421	case <-time.After(time.Second):
329	t.Fatal("Delete blocked after virtiofsd exit")	422	t.Fatal("Delete blocked after virtiofsd exit")
330	}	423	}
		424	+ select {
		425	+ case <-exitHandled:
		426	+ case <-time.After(time.Second):
		427	+ t.Fatal("exit handler did not finish after Delete")
		428	+ }
331	if cleanupCalls != 1 {	429	if cleanupCalls != 1 {
332	t.Fatalf("sandbox cleanup calls = %d, want 1", cleanupCalls)	430	t.Fatalf("sandbox cleanup calls = %d, want 1", cleanupCalls)
333	}	431	}
	@@ -337,14 +435,34 @@ func TestWaitForSandboxExitDoesNotDuplicateDeleteCleanup(t *testing.T) {
337	}	435	}
338		436
339	func newExitTestSandbox(cleanup func(context.Context) error) (Manager, sandboxEntry, *Sandbox) {	437	func newExitTestSandbox(cleanup func(context.Context) error) (Manager, sandboxEntry, *Sandbox) {
340	- m := &Manager{boot: &recordingBootPreparer{}, cidAllocator: NewCIDAllocator()}	438	+ m := &Manager{
341	- sbx := &Sandbox{cleanup: NewCleanup(), sandboxID: "sandbox-a"}	439	+ boot: &recordingBootPreparer{},
		440	+ cidAllocator: NewCIDAllocator(),
		441	+ }
		442	+ sbx := &Sandbox{process: &fakeSandboxProcess{}, cleanup: NewCleanup(), sandboxID: "sandbox-a"}
342	sbx.cleanup.Add(cleanup)	443	sbx.cleanup.Add(cleanup)
343	entry := &sandboxEntry{state: sandboxReady, sbx: sbx}	444	entry := &sandboxEntry{state: sandboxReady, sbx: sbx}
344	m.sandboxes.Store("sandbox-a", entry)	445	m.sandboxes.Store("sandbox-a", entry)
345	return m, entry, sbx	446	return m, entry, sbx
346	}	447	}
347		448
		449	+func TestDeleteKeepsEntryWhenVMMStopFails(t *testing.T) {
		450	+ m, entry, sbx := newExitTestSandbox(func(context.Context) error {
		451	+ t.Fatal("resource cleanup ran after VMM stop failed")
		452	+ return nil
		453	+ })
		454	+ wantErr := errors.New("stop failed")
		455	+ sbx.process = &fakeSandboxProcess{stopErr: wantErr}
		456	+
		457	+ err := m.Delete(DeleteRequest{SandboxID: "sandbox-a"})
		458	+ if !errors.Is(err, wantErr) {
		459	+ t.Fatalf("Delete() error = %v, want %v", err, wantErr)
		460	+ }
		461	+ if actual, ok := m.sandboxes.Load("sandbox-a"); !ok \|\| actual != entry {
		462	+ t.Fatal("sandbox entry was removed after VMM stop failure")
		463	+ }
		464	+}
		465	+
348	func TestCheckpointCapturesRunningAndSuspendedSandbox(t *testing.T) {	466	func TestCheckpointCapturesRunningAndSuspendedSandbox(t *testing.T) {
349	tests := []struct {	467	tests := []struct {
350	name string	468	name string

Minternal/sandbox/sandbox.go+19-14

	@@ -57,9 +57,22 @@ func vmStartSpecFromBootSpec(spec BootSpec) VMStartSpec {
57	}	57	}
58	}	58	}
59		59
		60	+// sandboxProcess exposes the VMM operations owned by the sandbox lifecycle.
		61	+type sandboxProcess interface {
		62	+ Wait() error
		63	+ Stop() error
		64	+ Pause(context.Context) error
		65	+ ResumeVM(context.Context) error
		66	+ CreateSnapshot(context.Context, string) error
		67	+ Pid() int
		68	+ SocketPath() string
		69	+}
		70	+
		71	+var _ sandboxProcess = (*vmm.Process)(nil)
		72	+
60	type Sandbox struct {	73	type Sandbox struct {
61	cleanup *Cleanup	74	cleanup *Cleanup
62	- process *vmm.Process	75	+ process sandboxProcess
63	vmStartSpec VMStartSpec	76	vmStartSpec VMStartSpec
64	vmmName string	77	vmmName string
65	sandboxID string	78	sandboxID string
	@@ -140,18 +153,13 @@ func RestoreSandbox(
140	}	153	}
141		154
142	cleanup.Add(func(ctx context.Context) error {	155	cleanup.Add(func(ctx context.Context) error {
143	- filesErr := cleanupFiles(sbx.process.VmmSocketPath, sbx.process.VsockSocketPath)	156	+ filesErr := cleanupFiles(vmmHandle.VmmSocketPath, vmmHandle.VsockSocketPath)
144	if filesErr != nil {	157	if filesErr != nil {
145	return fmt.Errorf("failed to cleanup files: %w", filesErr)	158	return fmt.Errorf("failed to cleanup files: %w", filesErr)
146	}	159	}
147		160
148	return nil	161	return nil
149	})	162	})
150	- cleanup.AddPriority(func(ctx context.Context) error {
151	- // Stop the sandbox first if it is still running, otherwise do nothing
152	- return sbx.Stop(ctx)
153	- })
154	-
155	return sbx, nil	163	return sbx, nil
156	}	164	}
157		165
	@@ -230,18 +238,13 @@ func CreateSandbox(
230	}	238	}
231		239
232	cleanup.Add(func(ctx context.Context) error {	240	cleanup.Add(func(ctx context.Context) error {
233	- filesErr := cleanupFiles(sbx.process.VmmSocketPath, sbx.process.VsockSocketPath)	241	+ filesErr := cleanupFiles(vmmHandle.VmmSocketPath, vmmHandle.VsockSocketPath)
234	if filesErr != nil {	242	if filesErr != nil {
235	return fmt.Errorf("failed to cleanup files: %w", filesErr)	243	return fmt.Errorf("failed to cleanup files: %w", filesErr)
236	}	244	}
237		245
238	return nil	246	return nil
239	})	247	})
240	- cleanup.AddPriority(func(ctx context.Context) error {
241	- // Stop the sandbox first if it is still running, otherwise do nothing
242	- return sbx.Stop(ctx)
243	- })
244	-
245	return sbx, nil	248	return sbx, nil
246	}	249	}
247		250
	@@ -258,7 +261,9 @@ func (s *Sandbox) Stop(ctx context.Context) error {
258	return nil	261	return nil
259	}	262	}
260		263
261	-func (s *Sandbox) Close(ctx context.Context) error {	264	+// CleanupResources releases resources owned by a sandbox whose VMM has stopped.
		265	+// Callers must confirm Stop succeeded (or observe VMM exit) before calling it.
		266	+func (s *Sandbox) CleanupResources(ctx context.Context) error {
262	err := s.cleanup.Run(ctx)	267	err := s.cleanup.Run(ctx)
263	if err != nil {	268	if err != nil {
264	return fmt.Errorf("failed to cleanup sandbox: %w", err)	269	return fmt.Errorf("failed to cleanup sandbox: %w", err)

Minternal/vmm/process.go+27-9

	@@ -12,6 +12,7 @@ import (
12	"strings"	12	"strings"
13	"sync"	13	"sync"
14	"syscall"	14	"syscall"
		15	+ "time"
15		16
16	"github.com/openeuler/Conch/internal/config"	17	"github.com/openeuler/Conch/internal/config"
17	"github.com/openeuler/Conch/pkg/ulog"	18	"github.com/openeuler/Conch/pkg/ulog"
	@@ -19,6 +20,7 @@ import (
19		20
20	const SocketDirPerm = 0755	21	const SocketDirPerm = 0755
21	const unixSocketPathMax = 107	22	const unixSocketPathMax = 107
		23	+const vmmStopTimeout = 10 * time.Second
22		24
23	// EnsureWorkSubDir creates a subdirectory under WorkDir and returns its path.	25	// EnsureWorkSubDir creates a subdirectory under WorkDir and returns its path.
24	func EnsureWorkSubDir(subDir string) (string, error) {	26	func EnsureWorkSubDir(subDir string) (string, error) {
	@@ -273,7 +275,6 @@ func getProcessState(pid int) (string, error) {
273		275
274	func (p *Process) Stop() error {	276	func (p *Process) Stop() error {
275	logger := ulog.GetLogger()	277	logger := ulog.GetLogger()
276	- var errs []error
277		278
278	if p.cmd == nil \|\| p.cmd.Process == nil {	279	if p.cmd == nil \|\| p.cmd.Process == nil {
279	p.adapter.Cleanup()	280	p.adapter.Cleanup()
	@@ -285,14 +286,18 @@ func (p *Process) Stop() error {
285	case <-p.exitDone:	286	case <-p.exitDone:
286	// Already exited	287	// Already exited
287	p.adapter.Cleanup()	288	p.adapter.Cleanup()
288	- return errors.Join(errs...)	289	+ return nil
289	default:	290	default:
290	}	291	}
291		292
292	if p.isAPIReady() {	293	if p.isAPIReady() {
293	if _, err := os.Stat(p.VmmSocketPath); err == nil {	294	if _, err := os.Stat(p.VmmSocketPath); err == nil {
294	if deleteErr := p.adapter.DeleteVM(); deleteErr != nil {	295	if deleteErr := p.adapter.DeleteVM(); deleteErr != nil {
295	- errs = append(errs, fmt.Errorf("delete vmm via api: %w", deleteErr))	296	+ // API failure does not prevent termination through SIGTERM.
		297	+ logger.Warn("failed to delete VMM via API, attempting SIGTERM",
		298	+ ulog.F("pid", p.cmd.Process.Pid),
		299	+ ulog.F("error", deleteErr),
		300	+ )
296	}	301	}
297	}	302	}
298	}	303	}
	@@ -309,30 +314,43 @@ func (p *Process) Stop() error {
309		314
310	err = p.cmd.Process.Signal(syscall.SIGTERM)	315	err = p.cmd.Process.Signal(syscall.SIGTERM)
311	if err != nil {	316	if err != nil {
		317	+ select {
		318	+ case <-p.exitDone:
		319	+ p.adapter.Cleanup()
		320	+ return nil
		321	+ default:
		322	+ }
312	if errors.Is(err, os.ErrProcessDone) {	323	if errors.Is(err, os.ErrProcessDone) {
313	logger.Debug("VMM process already exited",	324	logger.Debug("VMM process already exited",
314	ulog.F("pid", p.cmd.Process.Pid),	325	ulog.F("pid", p.cmd.Process.Pid),
315	)	326	)
316	p.adapter.Cleanup()	327	p.adapter.Cleanup()
317	- return errors.Join(errs...)	328	+ return nil
318	}	329	}
319	logger.Error("Failed to send SIGTERM to VMM process",	330	logger.Error("Failed to send SIGTERM to VMM process",
320	ulog.F("pid", p.cmd.Process.Pid),	331	ulog.F("pid", p.cmd.Process.Pid),
321	ulog.F("error", err),	332	ulog.F("error", err),
322	)	333	)
323	- errs = append(errs, fmt.Errorf("failed to send SIGTERM to vmm process, %d: %w", p.cmd.Process.Pid, err))	334	+ return fmt.Errorf("failed to send SIGTERM to vmm process, %d: %w", p.cmd.Process.Pid, err)
324	- return errors.Join(errs...)
325	}	335	}
326		336
327	logger.Debug("Sent SIGTERM to VMM process",	337	logger.Debug("Sent SIGTERM to VMM process",
328	ulog.F("pid", p.cmd.Process.Pid),	338	ulog.F("pid", p.cmd.Process.Pid),
329	)	339	)
330		340
331	- <-p.exitDone	341	+ timer := time.NewTimer(vmmStopTimeout)
332	- p.adapter.Cleanup()	342	+ defer timer.Stop()
333	- return errors.Join(errs...)	343	+ select {
		344	+ case <-p.exitDone:
		345	+ p.adapter.Cleanup()
		346	+ return nil
		347	+ case <-timer.C:
		348	+ return fmt.Errorf("timed out after %s waiting for VMM process %d to stop", vmmStopTimeout, p.cmd.Process.Pid)
		349	+ }
334	}	350	}
335		351
		352	+func (p *Process) SocketPath() string { return p.VmmSocketPath }
		353	+
336	func (p *Process) Pid() int {	354	func (p *Process) Pid() int {
337	if p.cmd == nil \|\| p.cmd.Process == nil {	355	if p.cmd == nil \|\| p.cmd.Process == nil {
338	logger := ulog.GetLogger()	356	logger := ulog.GetLogger()