Skip to content

AutoSession can attach a Session after concurrent Session.Close returns #249

Description

@linkdata

Summary

With Jaws.AutoSession enabled, a successful WebSocket upgrade can publish a new Session before attaching its Request. A concurrent caller can obtain that Session through Jaws.Sessions() and close it during this gap. The upgrade then attaches the already-closed Session to the Request.

This violates the lifecycle ordering around Session.Close:

  • If association wins first, Close must see the Request, detach it, and queue its reload.
  • If Close wins first, the Request must not subsequently associate with that Session.

Supported public interleaving

  1. Construct Jaws, enable AutoSession before serving, and start Serve.
  2. Create and claim a Request through NewRequest and UseRequest.
  3. Begin a valid WebSocket upgrade through Request.ServeHTTP.
  4. Once the auto-created Session becomes visible through Jaws.Sessions(), concurrently call Session.Close.
  5. Allow the upgrade to continue.

A ResponseWriter wrapper whose Header method waits after observing Jaws.Sessions() makes the interleaving deterministic. The observed result is:

after Close: session_count=0 max_age=-1 requests=0
ConnectFn: same_session=true session_count=0 max_age=-1 requests=1
handshake Set-Cookie="autosession=<id>; Path=/; HttpOnly; SameSite=Lax"
dial_error=<nil>

The handshake succeeds, but its Request points at an expired Session absent from the active-session registry. Because Close saw no associated Requests, the Request also misses the required reload.

Root cause

ensureAutoSession in request.go calls newSession before attaching the returned Session. newSession in session.go inserts the Session into jw.sessions, then invokes response/request cookie operations before returning. Jaws.Sessions therefore exposes it while its Request list is empty. After Session.Close deletes and expires it, ensureAutoSession attaches it without checking whether it is dead.

Suggested fix

Give AutoSession a creation path that establishes the initiating Request's membership before publishing the Session. Serialize that transition with Session.Close under the established lock order, release all internal locks before calling ResponseWriter, and ensure the emitted cookie reflects whichever transition wins.

Acceptance criteria

  • Add a deterministic regression test that closes an auto-created Session at the publication boundary of an accepted upgrade.
  • After Close returns, the closed Session is never subsequently installed in Request.Session or appended to Session.Requests.
  • If attach/publication wins first, Close detaches the Request and queues exactly one reload, including before WebSocket subscription.
  • If Close wins, the handshake does not emit a live cookie for the closed Session.
  • Rejected handshakes still create neither a Session nor a cookie.
  • Reentrant ResponseWriter.Header calls remain deadlock-free.
  • Run the regression and package suite under go test -race.

Related issues

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinggoPull requests that update go codeseverity:medium

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions