Refine sb.Terminate and .Detach

ehdr · ehdr · commit ac41d2707f9e · 2025-12-26T14:53:36.000+02:00
diff --git a/modal-go/sandbox.go b/modal-go/sandbox.go
@@ -716,23 +716,38 @@ func (sb *Sandbox) getOrCreateCommandRouterClient(ctx context.Context, taskID st
 	return sb.commandRouterClient, nil
 }
 
-// Close closes any resources associated with the Sandbox.
-// This should be called when the Sandbox is no longer needed
-// in the local client. The sandbox can still be running and
-// accessed in other clients.
-func (sb *Sandbox) Close() {
+// Detach disconnects from the running Sandbox, and closies the Stdin/Stdout/Stderr streams.
+// The remote Sandbox continues running and can be accessed from other clients.
+func (sb *Sandbox) Detach() {
 	sb.mu.Lock()
 	defer sb.mu.Unlock()
 
 	if sb.commandRouterClient != nil {
 		_ = sb.commandRouterClient.Close()
 		sb.commandRouterClient = nil
 	}
+
+	if sb.Stdin != nil {
+		_ = sb.Stdin.Close()
+	}
+	if sb.Stdout != nil {
+		_ = sb.Stdout.Close()
+	}
+	if sb.Stderr != nil {
+		_ = sb.Stderr.Close()
+	}
 }
 
 // Terminate stops the Sandbox.
+// The Stdin, Stdout, Stderr streams are not closed.
 func (sb *Sandbox) Terminate(ctx context.Context) error {
-	sb.Close()
+	sb.mu.Lock()
+	if sb.commandRouterClient != nil {
+		_ = sb.commandRouterClient.Close()
+		sb.commandRouterClient = nil
+	}
+	sb.mu.Unlock()
+
 	_, err := sb.client.cpClient.SandboxTerminate(ctx, pb.SandboxTerminateRequest_builder{
 		SandboxId: sb.SandboxID,
 	}.Build())
diff --git a/modal-go/test/sandbox_test.go b/modal-go/test/sandbox_test.go
@@ -1076,3 +1076,126 @@ func TestSandboxExecAfterTerminate(t *testing.T) {
 	_, err = sb.Exec(ctx, []string{"echo", "hello"}, nil)
 	g.Expect(err).To(gomega.HaveOccurred())
 }
+
+func TestSandboxDetachThenExec(t *testing.T) {
+	t.Parallel()
+	g := gomega.NewWithT(t)
+	ctx := context.Background()
+	tc := newTestClient(t)
+
+	app, err := tc.Apps.FromName(ctx, "libmodal-test", &modal.AppFromNameParams{CreateIfMissing: true})
+	g.Expect(err).ToNot(gomega.HaveOccurred())
+
+	image := tc.Images.FromRegistry("alpine:3.21", nil)
+
+	sb, err := tc.Sandboxes.Create(ctx, app, image, nil)
+	g.Expect(err).ToNot(gomega.HaveOccurred())
+	defer terminateSandbox(g, sb)
+
+	p1, err := sb.Exec(ctx, []string{"echo", "first"}, nil)
+	g.Expect(err).ToNot(gomega.HaveOccurred())
+	exitCode1, err := p1.Wait(ctx)
+	g.Expect(err).ToNot(gomega.HaveOccurred())
+	g.Expect(exitCode1).To(gomega.Equal(0))
+
+	sb.Detach()
+
+	// Second exec creates new command router client
+	p2, err := sb.Exec(ctx, []string{"echo", "second"}, nil)
+	g.Expect(err).ToNot(gomega.HaveOccurred())
+	exitCode2, err := p2.Wait(ctx)
+	g.Expect(err).ToNot(gomega.HaveOccurred())
+	g.Expect(exitCode2).To(gomega.Equal(0))
+}
+
+func TestSandboxDetachIsNonDestructive(t *testing.T) {
+	t.Parallel()
+	g := gomega.NewWithT(t)
+	ctx := context.Background()
+	tc := newTestClient(t)
+
+	app, err := tc.Apps.FromName(ctx, "libmodal-test", &modal.AppFromNameParams{CreateIfMissing: true})
+	g.Expect(err).ToNot(gomega.HaveOccurred())
+
+	image := tc.Images.FromRegistry("alpine:3.21", nil)
+
+	sb, err := tc.Sandboxes.Create(ctx, app, image, nil)
+	g.Expect(err).ToNot(gomega.HaveOccurred())
+	defer terminateSandbox(g, sb)
+
+	sandboxID := sb.SandboxID
+
+	sb.Detach()
+
+	sbFromID, err := tc.Sandboxes.FromID(ctx, sandboxID)
+	g.Expect(err).ToNot(gomega.HaveOccurred())
+	defer sbFromID.Detach()
+	g.Expect(sbFromID.SandboxID).To(gomega.Equal(sandboxID))
+
+	p, err := sbFromID.Exec(ctx, []string{"echo", "still running"}, nil)
+	g.Expect(err).ToNot(gomega.HaveOccurred())
+	exitCode, err := p.Wait(ctx)
+	g.Expect(err).ToNot(gomega.HaveOccurred())
+	g.Expect(exitCode).To(gomega.Equal(0))
+}
+
+func TestSandboxDetachIsIdempotent(t *testing.T) {
+	t.Parallel()
+	g := gomega.NewWithT(t)
+	ctx := context.Background()
+	tc := newTestClient(t)
+
+	app, err := tc.Apps.FromName(ctx, "libmodal-test", &modal.AppFromNameParams{CreateIfMissing: true})
+	g.Expect(err).ToNot(gomega.HaveOccurred())
+
+	image := tc.Images.FromRegistry("alpine:3.21", nil)
+
+	sb, err := tc.Sandboxes.Create(ctx, app, image, nil)
+	g.Expect(err).ToNot(gomega.HaveOccurred())
+	defer terminateSandbox(g, sb)
+
+	sb.Detach()
+	sb.Detach()
+	sb.Detach()
+}
+
+func TestSandboxDetachThenTerminate(t *testing.T) {
+	t.Parallel()
+	g := gomega.NewWithT(t)
+	ctx := context.Background()
+	tc := newTestClient(t)
+
+	app, err := tc.Apps.FromName(ctx, "libmodal-test", &modal.AppFromNameParams{CreateIfMissing: true})
+	g.Expect(err).ToNot(gomega.HaveOccurred())
+
+	image := tc.Images.FromRegistry("alpine:3.21", nil)
+
+	sb, err := tc.Sandboxes.Create(ctx, app, image, nil)
+	g.Expect(err).ToNot(gomega.HaveOccurred())
+	defer terminateSandbox(g, sb)
+
+	sb.Detach()
+
+	err = sb.Terminate(ctx)
+	g.Expect(err).ToNot(gomega.HaveOccurred())
+}
+
+func TestSandboxTerminateThenDetach(t *testing.T) {
+	t.Parallel()
+	g := gomega.NewWithT(t)
+	ctx := context.Background()
+	tc := newTestClient(t)
+
+	app, err := tc.Apps.FromName(ctx, "libmodal-test", &modal.AppFromNameParams{CreateIfMissing: true})
+	g.Expect(err).ToNot(gomega.HaveOccurred())
+
+	image := tc.Images.FromRegistry("alpine:3.21", nil)
+
+	sb, err := tc.Sandboxes.Create(ctx, app, image, nil)
+	g.Expect(err).ToNot(gomega.HaveOccurred())
+
+	err = sb.Terminate(ctx)
+	g.Expect(err).ToNot(gomega.HaveOccurred())
+
+	sb.Detach()
+}
diff --git a/modal-js/src/sandbox.ts b/modal-js/src/sandbox.ts
@@ -941,20 +941,30 @@ export class Sandbox {
   }
 
   /**
-   * Close any resources associated with the Sandbox.
-   * This should be called when the Sandbox is no longer needed
-   * in the local client. The sandbox can still be running and
-   * accessed in other clients.
+   * Detach from the running Sandbox, closing the stdin/stdout/stderr streams.
+   * The remote Sandbox continues running and can be accessed from other clients.
    */
-  close(): void {
+  detach(): void {
     if (this.#commandRouterClient) {
       this.#commandRouterClient.close();
       this.#commandRouterClient = undefined;
     }
+
+    this.stdin.abort().catch(() => {});
+    this.stdout.cancel().catch(() => {});
+    this.stderr.cancel().catch(() => {});
   }
 
+  /**
+   * Terminate the Sandbox.
+   * The stdin, stdout, stderr streams are not closed.
+   */
   async terminate(): Promise<void> {
-    this.close();
+    if (this.#commandRouterClient) {
+      this.#commandRouterClient.close();
+      this.#commandRouterClient = undefined;
+    }
+
     await this.#client.cpClient.sandboxTerminate({ sandboxId: this.sandboxId });
     this.#taskId = undefined; // Reset task ID after termination
   }
diff --git a/modal-js/test/sandbox.test.ts b/modal-js/test/sandbox.test.ts
@@ -1017,3 +1017,83 @@ test("SandboxExecAfterTerminate", async () => {
 
   await expect(sb.exec(["echo", "hello"])).rejects.toThrow();
 });
+
+test("SandboxDetachThenExec", async () => {
+  const app = await tc.apps.fromName("libmodal-test", {
+    createIfMissing: true,
+  });
+  const image = tc.images.fromRegistry("alpine:3.21");
+
+  const sb = await tc.sandboxes.create(app, image);
+  onTestFinished(async () => await sb.terminate());
+
+  const p1 = await sb.exec(["echo", "first"]);
+  const exitCode1 = await p1.wait();
+  expect(exitCode1).toBe(0);
+
+  sb.detach();
+
+  // Second exec creates new command router client
+  const p2 = await sb.exec(["echo", "second"]);
+  const exitCode2 = await p2.wait();
+  expect(exitCode2).toBe(0);
+});
+
+test("SandboxDetachIsNonDestructive", async () => {
+  const app = await tc.apps.fromName("libmodal-test", {
+    createIfMissing: true,
+  });
+  const image = tc.images.fromRegistry("alpine:3.21");
+
+  const sb = await tc.sandboxes.create(app, image);
+  onTestFinished(async () => await sb.terminate());
+
+  const sandboxId = sb.sandboxId;
+
+  sb.detach();
+
+  const sbFromId = await tc.sandboxes.fromId(sandboxId);
+  expect(sbFromId.sandboxId).toBe(sandboxId);
+
+  const p = await sbFromId.exec(["echo", "still running"]);
+  const exitCode = await p.wait();
+  expect(exitCode).toBe(0);
+});
+
+test("SandboxDetachIsIdempotent", async () => {
+  const app = await tc.apps.fromName("libmodal-test", {
+    createIfMissing: true,
+  });
+  const image = tc.images.fromRegistry("alpine:3.21");
+
+  const sb = await tc.sandboxes.create(app, image);
+  onTestFinished(async () => await sb.terminate());
+
+  sb.detach();
+  sb.detach();
+});
+
+test("SandboxDetachThenTerminate", async () => {
+  const app = await tc.apps.fromName("libmodal-test", {
+    createIfMissing: true,
+  });
+  const image = tc.images.fromRegistry("alpine:3.21");
+
+  const sb = await tc.sandboxes.create(app, image);
+  onTestFinished(async () => await sb.terminate());
+
+  sb.detach();
+  await sb.terminate();
+});
+
+test("SandboxTerminateThenDetach", async () => {
+  const app = await tc.apps.fromName("libmodal-test", {
+    createIfMissing: true,
+  });
+  const image = tc.images.fromRegistry("alpine:3.21");
+
+  const sb = await tc.sandboxes.create(app, image);
+
+  await sb.terminate();
+  sb.detach();
+});
