rdma: make the teardown callback the single RC publication owner

Round-3 review of the ownership model found that moving publication
between the request path and the callback by hand leaves edges where
a record is published twice or lost. The publication model is now
structural instead: request paths only record outcomes, and the
native teardown callback - which the ABI fires exactly once per
destroyed session, after every completion call - is the single
publisher.

The READY handler records its outcome (success with the reported
byte count, or the mapped failure) and lets the callback publish.
A deferred recorder covers panic unwinds, so every path through
the handler leaves a final outcome behind. A failed PREPARE
finalization publishes through a consume-or-noop helper: when the
finalizing call already reaped the session the entry is gone and
the helper is a no-op, otherwise it publishes the failure itself.

Unclaimed teardowns no longer all read as expiries: the record is
classified from the native outcome, so a transfer that died on the
wire, failed verification, or timed out carries its own code and
status.

The synthesized publication context runs on an immutable app, so
string accessors copy instead of exposing the pooled request buffer
to the asynchronously serializing event senders. Captured account
and region strings are cloned for the same reason. The nil-error
publication no longer asserts on the S3 error interface, and the
panic marker is a proper internal S3 error.

Unit tests cover the table semantics: expiry publication,
first-record-wins, consume-or-noop failure publication, unregister,
unknown sessions, error normalization, status mapping, and the
outcome classification.

rdma/rcroutes/ops_linux.go

@@ -19,6 +19,7 @@ package rcroutes
 
 import (
 	"errors"
+	"net/http"
 	"strings"
 	"sync"
 	"time"
@@ -64,9 +65,10 @@ type opsEmitter struct {
 // synthesize builds a fiber context whose path and request locals
 // describe the session's logical object operation, so the standard
 // access-log and event pipelines observe GET/PUT of bucket/key
-// instead of the fixed RDMA control path. The path string is the
-// emitter's own storage: event senders serialize asynchronously, so
-// the synthesized context must never hand them pooled buffers.
+// instead of the fixed RDMA control path. The app runs with
+// Immutable: string accessors copy instead of exposing the pooled
+// context buffer, which matters because event senders serialize
+// asynchronously and would otherwise read a reused buffer.
 func (e *opsEmitter) synthesize() (fiber.Ctx, func()) {
 	ctx := e.app.AcquireCtx(&fasthttp.RequestCtx{})
 	method := fiber.MethodGet
@@ -76,9 +78,7 @@ func (e *opsEmitter) synthesize() (fiber.Ctx, func()) {
 	ctx.Method(method)
 	// The access logger and the event schema both split this path
 	// into bucket/key, so the synthesized path must be the object
-	// path in canonical form. fiber copies override strings it
-	// stores as the path original; the derived c.path below is
-	// a fresh allocation, which is what outlives the release.
+	// path in canonical form.
 	ctx.Path("/" + e.bucket + "/" + e.key)
 	utils.ContextKeyAccount.Set(ctx, e.acct)
 	utils.ContextKeyRegion.Set(ctx, e.region)
@@ -94,22 +94,15 @@ func (e *opsEmitter) synthesize() (fiber.Ctx, func()) {
 // committed PUT) the object-created event. Exactly-once delivery is
 // the tracker's job; this method just performs one emission.
 //
-// The operational sinks classify plain errors as 500 on their own,
-// which would disagree with the status the client saw. Before the
-// record reaches them, the error is rendered as its mapped S3 error,
-// so the audit log, the metric, and the wire response all carry the
-// same classification.
+// The operational sinks classify plain errors as 500 on their own
+// and unwrap nothing, so the publication always hands them the
+// error in its normalized S3 form: the audit log, the metric, and
+// the wire response then carry the same classification.
 func (e *opsEmitter) publish(err error, bytes int64) {
 	if e == nil || (e.ops.Logger == nil && e.ops.Metrics == nil && e.ops.Events == nil) {
 		return
 	}
-	sinkErr := err
-	if err != nil {
-		var s3Err s3err.S3Error
-		if !errors.As(err, &s3Err) {
-			sinkErr = routeError(err)
-		}
-	}
+	sinkErr := normalizeSinkError(err)
 	ctx, release := e.synthesize()
 	defer release()
 
@@ -117,7 +110,10 @@ func (e *opsEmitter) publish(err error, bytes int64) {
 	if e.isPut {
 		action = metrics.ActionPutObject
 	}
-	status := httpStatusFromError(sinkErr)
+	status := http.StatusOK
+	if sinkErr != nil {
+		status = sinkErr.(s3err.APIError).HTTPStatusCode
+	}
 
 	if e.ops.Metrics != nil {
 		e.ops.Metrics.Send(ctx, sinkErr, action, bytes, status)
@@ -137,9 +133,26 @@ func (e *opsEmitter) publish(err error, bytes int64) {
 	}
 }
 
+// normalizeSinkError renders any operation error as the plain
+// s3err.APIError the sinks expect: wrapped S3 errors keep their
+// payload (the audit loggers assert the S3Error interface directly
+// and would misclassify a wrapper), and non-S3 errors map through
+// the same route error mapping the wire response uses.
+func normalizeSinkError(err error) error {
+	if err == nil {
+		return nil
+	}
+	var s3Err s3err.S3Error
+	if errors.As(err, &s3Err) {
+		return s3Err.BaseError()
+	}
+	return routeError(err)
+}
+
 // httpStatusFromError maps an operation error to the HTTP status
-// the S3 surface would have answered with. The callers pass mapped
-// S3 errors, so the status is simply the error's own.
+// the S3 surface would have answered with, using the same route
+// error mapping as the wire response so operational records never
+// disagree with what the client saw.
 func httpStatusFromError(err error) int {
 	if err == nil {
 		return 200
@@ -147,36 +160,42 @@ func httpStatusFromError(err error) int {
 	return routeError(err).HTTPStatusCode
 }
 
+// sessionOutcome is the terminal outcome of a tracked session,
+// recorded by whichever path observes the final result first. A
+// recorded outcome is final: the teardown callback consumes it and
+// publishes, so exactly one publication happens per session.
+type sessionOutcome struct {
+	err  error // nil on success
+	byt  int64 // bytes transferred on success
+	done bool  // outcome recorded
+}
+
 // sessionRecord is one tracked session with its captured context.
 type sessionRecord struct {
 	emit *opsEmitter
-	// reserved marks a record the request path holds exclusively:
-	// it took ownership before invoking a native completion call
-	// that would fire the teardown callback synchronously, so the
-	// callback must not publish on its behalf.
-	reserved bool
+	out  sessionOutcome
 }
 
-// opsTracker owns terminal publication: each session (and each
-// pre-session request) is published exactly once, by whichever path
-// confirms the final outcome first. It carries its own throwaway
-// fiber.App for synthesizing publication contexts, independent of
-// the gateway's request routing.
+// opsTracker owns terminal publication: each session publishes
+// exactly once. The request paths only ever RECORD an outcome; the
+// native teardown callback - which the ABI guarantees fires exactly
+// once per destroyed session, after every completion call - is the
+// single publisher. This removes every ownership race: a recorded
+// outcome cannot be double-published, and a record the callback
+// already consumed cannot be resurrected.
 type opsTracker struct {
 	mu       sync.Mutex
 	ops      OpsServices
 	sessions map[string]*sessionRecord
-	// earlyTerminals parks teardown notifications that arrived
-	// before the session's registration; register consumes them.
-	earlyTerminals map[string]rcserver.TerminalEvent
-	app            *fiber.App
+	app      *fiber.App
 }
 
 func newOpsTracker() *opsTracker {
 	return &opsTracker{
-		sessions:       map[string]*sessionRecord{},
-		earlyTerminals: map[string]rcserver.TerminalEvent{},
-		app:            fiber.New(),
+		sessions: map[string]*sessionRecord{},
+		app: fiber.New(fiber.Config{
+			Immutable: true,
+		}),
 	}
 }
 
@@ -192,23 +211,22 @@ func (t *opsTracker) SetOpsServices(ops OpsServices) {
 	t.ops = ops
 }
 
-// register captures the operational context of a successfully created
-// session so a later terminal path can publish its final outcome.
+// register captures the operational context of a successfully
+// created session so the terminal outcome can be published later.
 // The strings are cloned: they originate from the request's pooled
 // header buffer, which does not survive the response.
 //
-// handleEarlyTerminal covers the FinishPrepare race: the native call
-// that finalizes PREPARE can reap an already-expired session and fire
-// the teardown callback before register runs. When the callback wins
-// that race it parks the event, and register consumes it instead of
-// leaving an entry whose only notification already happened.
+// The account is captured by value but its string fields still
+// reference request storage on some IAM paths, so the sink-relevant
+// identity is cloned as well.
 func (t *opsTracker) register(sessionID string, acct auth.Account,
 	region, bucket, key string, isPut bool, start time.Time) {
+	acct.Access = strings.Clone(acct.Access)
 	emit := &opsEmitter{
 		ops:    t.loadOps(),
 		app:    t.app,
 		acct:   acct,
-		region: region,
+		region: strings.Clone(region),
 		bucket: strings.Clone(bucket),
 		key:    strings.Clone(key),
 		isPut:  isPut,
@@ -217,21 +235,14 @@ func (t *opsTracker) register(sessionID string, acct auth.Account,
 
 	t.mu.Lock()
 	defer t.mu.Unlock()
-	// A teardown notification that arrived before this registration
-	// owns the publication: publish now and store nothing.
-	if _, parked := t.earlyTerminals[sessionID]; parked {
-		delete(t.earlyTerminals, sessionID)
-		go emit.publish(errSessionExpired, 0)
-		return
-	}
 	t.sessions[sessionID] = &sessionRecord{emit: emit}
 }
 
 // unregister drops a session entry whose PREPARE finalization
-// failed: the native side is gone, so the teardown callback has
-// either already published or will find nothing. A parked early
-// notification is dropped with it (the failure publication covers
-// the outcome).
+// failed before the session was committed: the native side either
+// rejected it (no callback will come) or already reaped it (the
+// callback found no record and published nothing). The failure
+// itself is published as a request record by the caller.
 func (t *opsTracker) unregister(sessionID string) {
 	if t == nil {
 		return
@@ -239,45 +250,46 @@ func (t *opsTracker) unregister(sessionID string) {
 	t.mu.Lock()
 	defer t.mu.Unlock()
 	delete(t.sessions, sessionID)
-	delete(t.earlyTerminals, sessionID)
 }
 
-// reserve takes exclusive ownership of a session's publication
-// before the request path invokes a native completion call
-// (FinishFinal, FinishPut, or a reap-triggering mutation). Those
-// calls fire the teardown callback synchronously while the session
-// record is still live; reserving first keeps the callback from
-// publishing an expiry record for a transfer that is completing
-// right now.
-func (t *opsTracker) reserve(sessionID string) *opsEmitter {
-	t.mu.Lock()
-	defer t.mu.Unlock()
-	rec, ok := t.sessions[sessionID]
-	if !ok || rec.reserved {
-		return nil
-	}
-	rec.reserved = true
-	return rec.emit
-}
-
-// unreserve restores callback ownership when a reserved completion
-// call did not after all retire the session (the caller failed
-// before any state change). The record goes back to normal tracking
-// unless a teardown notification landed meanwhile.
-func (t *opsTracker) unreserve(sessionID string, emit *opsEmitter) {
-	if t == nil || emit == nil {
+// failOutcome publishes a failed finalization exactly once: when
+// the finalizing call already reaped the session its callback
+// published (the entry is gone, this is a no-op); when no callback
+// will ever come (the native side rejected the call) the entry is
+// consumed and published here.
+func (t *opsTracker) failOutcome(sessionID string, err error) {
+	if t == nil {
 		return
 	}
 	t.mu.Lock()
-	defer t.mu.Unlock()
 	rec, ok := t.sessions[sessionID]
+	if ok {
+		delete(t.sessions, sessionID)
+	}
+	t.mu.Unlock()
 	if !ok {
-		// The session is gone: the completion call retired it and
-		// the reserved emitter is the only remaining owner, so
-		// nothing to restore.
 		return
 	}
-	rec.reserved = false
+	rec.emit.publish(err, 0)
+}
+
+// recordOutcome stores the terminal outcome of a completing
+// transfer. It never publishes: publication belongs to the teardown
+// callback, which fires exactly once per session after the
+// completion call returns. Recording is idempotent - the first
+// outcome wins - so a panic-unwind path that records after the
+// normal path changed nothing.
+func (t *opsTracker) recordOutcome(sessionID string, err error, bytes int64) {
+	if t == nil {
+		return
+	}
+	t.mu.Lock()
+	defer t.mu.Unlock()
+	rec, ok := t.sessions[sessionID]
+	if !ok || rec.out.done {
+		return
+	}
+	rec.out = sessionOutcome{err: err, byt: bytes, done: true}
 }
 
 func (t *opsTracker) loadOps() OpsServices {
@@ -286,65 +298,48 @@ func (t *opsTracker) loadOps() OpsServices {
 	return t.ops
 }
 
-// claim removes the session from the table and returns its emitter
-// to exactly one publisher. A reserved record is only claimable by
-// its reserving request path (the callback skips it).
-func (t *opsTracker) claim(sessionID string, byRequest bool) *opsEmitter {
-	t.mu.Lock()
-	defer t.mu.Unlock()
-	rec, ok := t.sessions[sessionID]
-	if !ok {
-		return nil
-	}
-	if rec.reserved && !byRequest {
-		return nil
-	}
-	delete(t.sessions, sessionID)
-	return rec.emit
-}
-
-// onTerminal is the native teardown callback path: publish sessions
-// that no handler ever claimed (expired, abandoned, or canceled
-// without a READY).
+// onTerminal is the native teardown callback: the single publisher
+// of session records. It consumes the recorded outcome (success,
+// failure, or expiry when no outcome was ever recorded) and removes
+// the entry, so exactly one publication happens per session no
+// matter which path confirmed the result.
 func (t *opsTracker) onTerminal(ev rcserver.TerminalEvent) {
 	if t == nil {
 		return
 	}
-	emit := t.claim(ev.SessionID, false)
-	if emit != nil {
-		// An expired or abandoned session never reached a final
-		// object result; the bytes staged for it did not become
-		// a transfer.
-		emit.publish(errSessionExpired, 0)
+	t.mu.Lock()
+	rec, ok := t.sessions[ev.SessionID]
+	if !ok {
+		t.mu.Unlock()
 		return
 	}
-	// A session tearing down before its registration ran: park the
-	// event so register can publish instead of orphaning a record
-	// whose only notification already happened.
-	t.mu.Lock()
-	if _, live := t.sessions[ev.SessionID]; !live {
-		t.earlyTerminals[ev.SessionID] = ev
-	}
+	delete(t.sessions, ev.SessionID)
 	t.mu.Unlock()
+
+	if !rec.out.done {
+		// No request path ever confirmed a result: the session
+		// expired, was abandoned, or was canceled. The event's
+		// outcome carries the native reason.
+		rec.out = sessionOutcome{err: expiredError(ev), done: true}
+	}
+	emit := rec.emit
+	emit.publish(rec.out.err, rec.out.byt)
 }
 
-// publishClaimed publishes the terminal record from the request
-// path (READY completion or failure). A nil tracker (no operational
-// services) and an unknown session (already claimed or never
-// registered) are both silent no-ops.
-func (t *opsTracker) publishClaimed(sessionID string, err error, bytes ...int64) {
-	if t == nil {
-		return
+// expiredError renders an unclaimed teardown as the error the
+// publication carries, derived from the native outcome so the
+// record names the real terminal reason instead of a generic one.
+func expiredError(ev rcserver.TerminalEvent) error {
+	switch ev.Outcome {
+	case int(rcserver.ReadyWireFail):
+		return rcserver.ErrWire
+	case int(rcserver.ReadyTimeout):
+		return errTimeoutExpired
+	case int(rcserver.ReadyVerifyFail):
+		return errVerifyFailed
+	default:
+		return errSessionExpired
 	}
-	emit := t.claim(sessionID, true)
-	if emit == nil {
-		return
-	}
-	var n int64
-	if len(bytes) > 0 {
-		n = bytes[0]
-	}
-	emit.publish(err, n)
 }
 
 // publishRequest emits an operation record for a request that ended
@@ -355,11 +350,12 @@ func (t *opsTracker) publishRequest(ctx fiber.Ctx, acct auth.Account,
 	if t == nil {
 		return
 	}
+	acct.Access = strings.Clone(acct.Access)
 	emit := &opsEmitter{
 		ops:    t.loadOps(),
 		app:    t.app,
 		acct:   acct,
-		region: regionFromCtx(ctx),
+		region: strings.Clone(regionFromCtx(ctx)),
 		bucket: strings.Clone(bucket),
 		key:    strings.Clone(key),
 		isPut:  isPut,
@@ -377,10 +373,9 @@ func regionFromCtx(ctx fiber.Ctx) string {
 	return ""
 }
 
-// expiredOutcomeError renders a parked teardown notification as the
-// error the publication carries: an internal S3 error whose code
-// names the expiry, so the audit log keeps the descriptive code the
-// plain error used to carry instead of the generic mapping.
+// sessionExpiredError is the S3 error an expired or abandoned
+// session publishes: an internal error whose code names the
+// expiry, so the audit log keeps a descriptive code.
 type sessionExpiredError struct {
 	s3err.APIError
 }
@@ -390,3 +385,19 @@ var errSessionExpired = sessionExpiredError{APIError: s3err.APIError{
 	Description:    "The RDMA transfer session expired before completion",
 	HTTPStatusCode: 500,
 }}
+
+// errTimeoutExpired and errVerifyFailed classify unclaimed teardowns
+// whose native outcome names a specific READY failure: the record
+// then matches what the wire response for that failure carries.
+var (
+	errTimeoutExpired = sessionExpiredError{APIError: s3err.APIError{
+		Code:           "RdmaTransferTimeout",
+		Description:    "The RDMA transfer timed out before completion",
+		HTTPStatusCode: 504,
+	}}
+	errVerifyFailed = sessionExpiredError{APIError: s3err.APIError{
+		Code:           "RdmaTransferVerifyFailed",
+		Description:    "The RDMA transfer failed verification before completion",
+		HTTPStatusCode: 502,
+	}}
+)

rdma/rcroutes/ops_linux_test.go

@@ -34,12 +34,11 @@ import (
 	"github.com/versity/versitygw/s3err"
 )
 
-// The tests exercise the tracker's ownership semantics: the session
-// table is the single source of truth for who may publish, so the
-// assertions watch table membership rather than the emission itself
-// (emission is a no-op without operational services wired in).
+// The publication model: request paths only record outcomes; the
+// teardown callback is the single publisher. The tests exercise
+// the table semantics that guarantee exactly-once publication.
 
-func TestOpsTrackerExpiryPublication(t *testing.T) {
+func TestOpsTrackerCallbackPublishesExpiry(t *testing.T) {
 	tr := newOpsTracker()
 	tr.register("sess-1", auth.Account{Access: "ak"}, "us-east-1",
 		"bkt", "obj", false, time.Now())
@@ -47,7 +46,8 @@ func TestOpsTrackerExpiryPublication(t *testing.T) {
 		t.Fatalf("registered sessions = %d, want 1", got)
 	}
 
-	// The reaper path claims and removes the session.
+	// The reaper path publishes for a session no READY ever
+	// recorded and removes the entry.
 	tr.onTerminal(rcserver.TerminalEvent{SessionID: "sess-1"})
 	if got := len(tr.sessions); got != 0 {
 		t.Fatalf("session survived terminal: %d", got)
@@ -60,102 +60,92 @@ func TestOpsTrackerExpiryPublication(t *testing.T) {
 	}
 }
 
-func TestOpsTrackerRequestPathClaims(t *testing.T) {
+func TestOpsTrackerRecordedOutcomeWins(t *testing.T) {
 	tr := newOpsTracker()
 	tr.register("sess-2", auth.Account{Access: "ak"}, "us-east-1",
 		"bkt", "obj", true, time.Now())
 
-	// READY completion claims the publication first.
-	tr.publishClaimed("sess-2", nil, 4096)
-	if got := len(tr.sessions); got != 0 {
-		t.Fatalf("request path left session: %d", got)
-	}
-
-	// The late reaper callback finds nothing left.
+	// The READY path records the real outcome; the callback then
+	// consumes it instead of publishing an expiry.
+	tr.recordOutcome("sess-2", nil, 4096)
 	tr.onTerminal(rcserver.TerminalEvent{SessionID: "sess-2"})
 	if got := len(tr.sessions); got != 0 {
-		t.Fatalf("reaper re-added session: %d", got)
+		t.Fatalf("session survived terminal: %d", got)
 	}
+
+	// A late second record is a no-op (entry gone).
+	tr.recordOutcome("sess-2", nil, 1)
 }
 
-func TestOpsTrackerReserveBlocksCallback(t *testing.T) {
+func TestOpsTrackerFirstRecordWins(t *testing.T) {
 	tr := newOpsTracker()
 	tr.register("sess-3", auth.Account{Access: "ak"}, "us-east-1",
-		"bkt", "obj", true, time.Now())
-
-	// The request path reserves before invoking a native
-	// completion call.
-	emit := tr.reserve("sess-3")
-	if emit == nil {
-		t.Fatal("reserve returned nil for a live session")
-	}
-
-	// The synchronous teardown callback must not publish on a
-	// reserved record: the entry stays put.
-	tr.onTerminal(rcserver.TerminalEvent{SessionID: "sess-3"})
-	if got := len(tr.sessions); got != 1 {
-		t.Fatalf("reserved session removed by callback: %d", got)
-	}
-
-	// A double reserve is refused.
-	if tr.reserve("sess-3") != nil {
-		t.Fatal("double reserve succeeded")
-	}
-
-	// The request path publishes through its reserved emitter.
-	tr.publishClaimed("sess-3", nil, 128)
-	if got := len(tr.sessions); got != 0 {
-		t.Fatalf("reserved session survived request publication: %d", got)
-	}
-}
-
-func TestOpsTrackerEarlyTerminal(t *testing.T) {
-	tr := newOpsTracker()
-
-	// Teardown notification for an unregistered session parks.
-	tr.onTerminal(rcserver.TerminalEvent{SessionID: "sess-4"})
-	if got := len(tr.earlyTerminals); got != 1 {
-		t.Fatalf("early terminal not parked: %d", got)
-	}
-
-	// Registration consumes it: no live entry remains, so no
-	// orphan record can outlive the session.
-	tr.register("sess-4", auth.Account{Access: "ak"}, "us-east-1",
 		"bkt", "obj", false, time.Now())
-	if got := len(tr.earlyTerminals); got != 0 {
-		t.Fatalf("early terminal not consumed: %d", got)
-	}
-	if got := len(tr.sessions); got != 0 {
-		t.Fatalf("orphan session entry created: %d", got)
+
+	tr.recordOutcome("sess-3", nil, 100)
+	tr.recordOutcome("sess-3", errors.New("late"), 0)
+
+	tr.mu.Lock()
+	rec := tr.sessions["sess-3"]
+	tr.mu.Unlock()
+	if rec == nil || rec.out.err != nil || rec.out.byt != 100 {
+		t.Fatalf("second record overwrote the first: %+v", rec.out)
 	}
 }
 
 func TestOpsTrackerUnregister(t *testing.T) {
 	tr := newOpsTracker()
-	tr.register("sess-5", auth.Account{Access: "ak"}, "us-east-1",
+	tr.register("sess-4", auth.Account{Access: "ak"}, "us-east-1",
 		"bkt", "obj", false, time.Now())
-	tr.onTerminal(rcserver.TerminalEvent{SessionID: "sess-5"})
-	tr.register("sess-6", auth.Account{Access: "ak"}, "us-east-1",
-		"bkt", "obj", false, time.Now())
-	tr.unregister("sess-6")
+	tr.unregister("sess-4")
 	if got := len(tr.sessions); got != 0 {
 		t.Fatalf("unregister left entries: %d", got)
 	}
+	// The teardown callback for the unregistered session is a
+	// silent no-op (native side already rejected or reaped it).
+	tr.onTerminal(rcserver.TerminalEvent{SessionID: "sess-4"})
+	if got := len(tr.sessions); got != 0 {
+		t.Fatalf("terminal resurrected entry: %d", got)
+	}
 }
 
 func TestOpsTrackerUnknownSession(t *testing.T) {
 	tr := newOpsTracker()
 	// Unknown sessions and the nil tracker are silent no-ops.
 	var nilTracker *opsTracker
-	nilTracker.publishClaimed("ghost", nil, 1)
+	nilTracker.recordOutcome("ghost", nil, 1)
 	nilTracker.onTerminal(rcserver.TerminalEvent{SessionID: "ghost"})
-	tr.publishClaimed("ghost", nil, 1)
+	tr.recordOutcome("ghost", nil, 1)
 	tr.onTerminal(rcserver.TerminalEvent{SessionID: "ghost"})
 	if got := len(tr.sessions); got != 0 {
 		t.Fatalf("ghost session materialized: %d", got)
 	}
 }
 
+func TestNormalizeSinkError(t *testing.T) {
+	if normalizeSinkError(nil) != nil {
+		t.Fatal("nil error should stay nil")
+	}
+	// Plain errors map through the route error mapping.
+	got := normalizeSinkError(errors.New("x"))
+	apiErr, ok := got.(s3err.APIError)
+	if !ok || apiErr.HTTPStatusCode != 500 {
+		t.Fatalf("plain error => %#v", got)
+	}
+	// Wrapped S3 errors are extracted to their base form so the
+	// audit loggers' direct assertion classifies them correctly.
+	wrapped := errWrapped{s3err.GetAPIError(s3err.ErrNoSuchBucket)}
+	got = normalizeSinkError(wrapped)
+	apiErr, ok = got.(s3err.APIError)
+	if !ok || apiErr.Code != "NoSuchBucket" || apiErr.HTTPStatusCode != 404 {
+		t.Fatalf("wrapped error => %#v", got)
+	}
+}
+
+type errWrapped struct{ s3err.S3Error }
+
+func (errWrapped) Error() string { return "wrapped" }
+
 func TestHttpStatusFromError(t *testing.T) {
 	if got := httpStatusFromError(nil); got != 200 {
 		t.Fatalf("nil error => %d, want 200", got)
@@ -172,3 +162,25 @@ func TestHttpStatusFromError(t *testing.T) {
 		t.Fatalf("limit error => %d, want 429", got)
 	}
 }
+
+func TestExpiredErrorClassification(t *testing.T) {
+	cases := []struct {
+		outcome int
+		code    string
+		status  int
+	}{
+		{int(rcserver.ReadyWireFail), "RdmaTransferFailed", 502},
+		{int(rcserver.ReadyVerifyFail), "RdmaTransferVerifyFailed", 502},
+		{int(rcserver.ReadyTimeout), "RdmaTransferTimeout", 504},
+		{int(rcserver.ReadyOK), "SessionExpired", 500},
+	}
+	for _, tc := range cases {
+		err := expiredError(rcserver.TerminalEvent{Outcome: tc.outcome})
+		apiErr := normalizeSinkError(err).(s3err.APIError)
+		if apiErr.Code != tc.code || apiErr.HTTPStatusCode != tc.status {
+			t.Fatalf("outcome %d => %s/%d, want %s/%d",
+				tc.outcome, apiErr.Code, apiErr.HTTPStatusCode,
+				tc.code, tc.status)
+		}
+	}
+}

rdma/rcroutes/routes_linux.go

@@ -42,6 +42,7 @@ import (
 	"github.com/versity/versitygw/backend"
 	"github.com/versity/versitygw/rdma/rcserver"
 	"github.com/versity/versitygw/s3api/utils"
+	"github.com/versity/versitygw/s3err"
 	"github.com/versity/versitygw/s3response"
 )
 
@@ -232,11 +233,13 @@ func (h *Handler) prepareCore(ctx fiber.Ctx) error {
 	h.ops.register(resp.SessionID, acct,
 		regionFromCtx(ctx), bucket, key, isPut, time.Now())
 	if err := h.svc.FinishPrepare(resp.SessionID, true); err != nil {
-		// The finalization failed: the session is gone and the
-		// teardown callback (or this call's own reap) published
-		// the terminal record; nothing may keep the entry.
-		h.ops.unregister(resp.SessionID)
-		h.ops.publishRequest(ctx, acct, mapRcError(err), bucket, key, isPut)
+		// The finalization failed. Exactly one publication
+		// covers it: the finalizing call already reaped the
+		// session and its callback published the recorded
+		// outcome, or the native side rejected the call and no
+		// callback is coming, in which case the entry is
+		// published here.
+		h.ops.failOutcome(resp.SessionID, mapRcError(err))
 		return mapRcError(err)
 	}
 
@@ -417,21 +420,27 @@ func (h *Handler) readyCore(ctx fiber.Ctx) error {
 	// this handler owns the completion ref. A panic or early
 	// unwind must still release it so the session can be reaped.
 	//
-	// The publication ownership moves here as well: every native
-	// completion call below (FinishFinal, FinishPut) fires the
-	// teardown callback synchronously, and a reserved record is
-	// invisible to that callback, so the outcome is published by
-	// this handler exactly once.
-	emit := h.ops.reserve(sessionID)
-	publish := func(err error, bytes int64) {
-		if emit != nil {
-			emit.publish(err, bytes)
-			emit = nil
+	// This handler only RECORDS the outcome; publication belongs
+	// to the teardown callback, which every native completion
+	// call below fires exactly once. The deferred recorder covers
+	// panic unwinds too, so every path through this handler
+	// leaves a final outcome behind.
+	recorded := false
+	record := func(err error, bytes int64) {
+		if !recorded {
+			recorded = true
+			h.ops.recordOutcome(sessionID, err, bytes)
 		}
 	}
 	finalized := false
 	defer func() {
 		if !finalized {
+			// The unwind finalizer retires the session, which
+			// fires the callback; make sure it carries an
+			// outcome even on a panic path.
+			if !recorded {
+				record(errPanicked(), 0)
+			}
 			_ = h.svc.FinishFinal(sessionID)
 		}
 	}()
@@ -449,7 +458,7 @@ func (h *Handler) readyCore(ctx fiber.Ctx) error {
 			finalized = true
 		}
 		if err != nil {
-			publish(mapRcError(err), 0)
+			record(mapRcError(err), 0)
 			return err
 		}
 		// The FINAL wire reply carries the stored object's
@@ -457,15 +466,19 @@ func (h *Handler) readyCore(ctx fiber.Ctx) error {
 		resp.Etag = put.ETag
 		resp.VersionID = put.VersionID
 	} else if err := h.svc.FinishFinal(sessionID); err != nil {
-		publish(mapRcError(err), 0)
+		record(mapRcError(err), 0)
 		return mapRcError(err)
 	} else {
 		finalized = true
 	}
 
-	// The transfer completed: publish the terminal record with
-	// the byte count the data plane reported.
-	publish(nil, int64(resp.BytesTransferred))
+	// The transfer completed: record the terminal outcome with
+	// the byte count the data plane reported. FinishPut (inside
+	// commitPut) or the FinishFinal above already fired the
+	// teardown callback, which publishes this outcome - or, on a
+	// timing edge where the callback ran first, the deferred
+	// finalizer's FinishFinal does.
+	record(nil, int64(resp.BytesTransferred))
 
 	// Wire reply per the hipobj-rc-v2 contract: protocol echo,
 	// cookie echo, transferred bytes, and object metadata.
@@ -699,3 +712,14 @@ func mapRcError(err error) error {
 	}
 	return err
 }
+
+// errPanicked is the outcome recorded when a panic unwinds the
+// READY handler after the completion ref was claimed: the record
+// keeps the failure even though the panic itself propagates.
+func errPanicked() error {
+	return s3err.APIError{
+		Code:           "InternalRDMAError",
+		Description:    "The RDMA transfer ended without a confirmed result",
+		HTTPStatusCode: 500,
+	}
+}