diff --git a/docs/file-access-protocol.md b/docs/file-access-protocol.md index de6b6553..c3123749 100644 --- a/docs/file-access-protocol.md +++ b/docs/file-access-protocol.md @@ -24,6 +24,8 @@ The Go client is `sandboxfs.NewClient(stream)`. It has one method per operation, Cancelling a call's context returns at once with `Cancelled` or `DeadlineExceeded`. A call cancelled before its request is written fails with `EffectNone`. After the request is written, the client sends `CancelRequest` for it and discards the late response, and the failure is `EffectNone` for a [side-effect-free request](#effects-and-cancellation) and `EffectPossible` for every other one. Cancelling a call while its request is being written fails the stream instead, because a partial frame cannot be withdrawn. A cancel that races the completion of a write may still fail the stream, and the requests in flight then fail with `EffectPossible`. +To learn what a cancelled request did, interrupt it instead: a call whose context comes from `sandboxfs.WithInterrupt` sends `CancelRequest` when the interrupt channel closes and keeps waiting for the request's own response, so it returns the request's result or the service's failure with its effect. A FUSE frontend uses this for a waiting lock, which may be acquired just before the cancellation arrives. + When the stream fails, every request in flight fails with `Unknown` and `EffectPossible`, and later calls fail with `EffectNone`; `errors.Is(err, sandboxfs.ErrTransport)` matches both. `Client.Done` closes and `Client.Err` returns the cause. To continue, open a new stream for the same attachment with `ExpectedServerInstanceID` set, as the [Sandbox link protocol](sandbox-link-protocol.md) describes. `InstanceChanged` then means every node, handle and lock of the attachment is gone. ## Implement a service diff --git a/internal/sandboxfs/client.go b/internal/sandboxfs/client.go index 7e41e393..6d29da43 100644 --- a/internal/sandboxfs/client.go +++ b/internal/sandboxfs/client.go @@ -246,17 +246,26 @@ func roundTrip[R message](ctx context.Context, c *Client, q Request) (R, error) return zero, fail } var out outcome - select { - case out = <-cl.ch: - case <-ctx.Done(): - if c.abandon(id, cl) { - effect := sandboxwire.EffectPossible - if sideEffectFree(q) { - effect = sandboxwire.EffectNone + interrupt, _ := ctx.Value(interruptKey{}).(<-chan struct{}) +wait: + for { + select { + case out = <-cl.ch: + break wait + case <-interrupt: + interrupt = nil + go c.cancel(id) + case <-ctx.Done(): + if c.abandon(id, cl) { + effect := sandboxwire.EffectPossible + if sideEffectFree(q) { + effect = sandboxwire.EffectNone + } + return zero, contextFailure(ctx.Err(), effect) } - return zero, contextFailure(ctx.Err(), effect) + out = <-cl.ch + break wait } - out = <-cl.ch } if out.fail != nil { return zero, out.fail @@ -264,6 +273,16 @@ func roundTrip[R message](ctx context.Context, c *Client, q Request) (R, error) return out.msg.(R), nil } +type interruptKey struct{} + +// WithInterrupt returns a context whose calls send CancelRequest once +// interrupt closes and keep waiting for the request's own response, so a call +// returns the request's real outcome: its result, or the service's failure +// with its effect. Ending the context still abandons the call. +func WithInterrupt(ctx context.Context, interrupt <-chan struct{}) context.Context { + return context.WithValue(ctx, interruptKey{}, interrupt) +} + func contextFailure(err error, effect sandboxwire.Effect) *Failure { code := CodeCancelled if errors.Is(err, context.DeadlineExceeded) { diff --git a/internal/sandboxfs/client_test.go b/internal/sandboxfs/client_test.go index ee4534a0..8453bb02 100644 --- a/internal/sandboxfs/client_test.go +++ b/internal/sandboxfs/client_test.go @@ -31,3 +31,27 @@ func TestCancelAfterWriteKeepsStream(t *testing.T) { t.Fatalf("describe after the cancellation: %v; stream: %v", err, c.Err()) } } + +// finisher completes Describe after its request is cancelled, as a lock +// acquired just before CancelRequest arrives does. +type finisher struct{ Service } + +func (finisher) Describe(ctx context.Context, _ Attachment, _ *DescribeRequest) (*DescribeResponse, error) { + <-ctx.Done() + return &DescribeResponse{ServerInstanceID: testInstance, Capabilities: testCaps}, nil +} + +// An interrupt cancels the request and still returns its own outcome. +func TestInterruptReturnsOutcome(t *testing.T) { + cc, sc := net.Pipe() + a := Attachment{ID: sandboxwire.NewID(), ServerInstanceID: testInstance, Lease: context.Background(), Exports: []sandboxlink.ExportGrant{{ID: "world"}}} + go Serve(context.Background(), sc, finisher{}, a) + c := NewClient(cc) + defer c.Close() + + interrupt := make(chan struct{}) + close(interrupt) + if _, err := c.Describe(WithInterrupt(context.Background(), interrupt), &DescribeRequest{}); err != nil { + t.Fatalf("interrupted describe that completed: %v", err) + } +}