fix(plugin-mssql): make connect cancellation prompt and Kerberos-safe#1892
Conversation
…t-cancel # Conflicts: # CHANGELOG.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f69050b39d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if gate.win(()) { | ||
| adopt(proc) |
There was a problem hiding this comment.
Adopt the dbproc before resuming connect
When dbopen succeeds, gate.win(()) resumes the awaiting connect() before dbproc is stored. If the caller immediately cancels/invalidates the attempt and calls disconnect() in that small window, disconnect() still sees a nil handle and returns, after which this block calls adopt(proc) and leaves a live connection marked connected after the session was torn down. Record/adopt the handle before resuming the caller, or otherwise make disconnect() see and close the pending proc.
Useful? React with 👍 / 👎.
Make SQL Server connect cancellation prompt, and Kerberos-safe
Closes #1889. Split out from the Windows Authentication work in #1879.
Problem
FreeTDSConnection.connect()awaited a bare continuation around a blockingdbopen()with no cancellation handler. The app does callTask.cancel()on the connect task, but cooperative cancellation can't resume a blocked continuation, so Cancel didn't return control to the UI untildbopenfinished on its own. Windows Authentication widened the window: the GSS ticket acquisition and the KDC round trip aren't bounded by the login timeout, so an unreachable KDC could hang for a long time.Separately, the Kerberos gate set
KRB5CCNAME, destroyed the ticket cache, and released its lock indefers scoped toawait conn.connect(). Making connect return early would have raced those cleanups against the still-running backgrounddbopen, which could clobber another connect's credentials.Fix
FreeTDS db-lib has no pollable connect (unlike libpq's
PQconnectStart/PQconnectPoll), so instead of forking FreeTDS this uses the native "resume the caller on cancel, let the blocking call finish and tear itself down" pattern:SingleResumeGate+runCancellableBlocking(new, Foundation-only inTableProMSSQLCore): a resume-exactly-once continuation box that races work completion, cancellation, and an app-owned deadline. The blocking work stays on its own serial queue, never the cooperative pool.FreeTDSConnection.connect()uses the gate.openConnection()returns thedbproc; the winner adopts it, the loserdbcloses it. An app-owned deadline ofloginTimeout + 5son a separate queue backstops a wedgeddbopen.KRB5CCNAMEset/restore, cache destroy, and process-wide serialization move inside the queue block wrappingdbopen, so they track its real completion rather than the early return. The async-lockMSSQLKerberosConnectGateis deleted; ticket acquisition (MSSQLKerberosCredentials) is now a cancellable, deadline-bounded call that discards an orphaned cache.connectionTimedOut(isKerberos:)): a plain TCP timeout message, or for Windows Auth one that points at the usual Kerberos causes (KDC unreachable, missing SPN, clock skew) and suggests SQL auth.MSSQLDriver) inherits the prompt return and handles the new error case. The CLAUDE.md "Cancelling a connect does not stop the driver" invariant now documents the non-pollable pattern so no one later assumes a fork was required.What this does not change
The app-level machinery (
ConnectionAttemptRegistry+ensureAttemptIsCurrent) is already generic and correct for MSSQL; it just could not run untilconnect()returned. Makingconnect()return promptly is what lets that safety net fire on time. iOS still has no attempt-registry equivalent, which is a separate gap I noted but did not fix here.Testing
TableProMSSQLCorebuilds; 27 tests pass, including: cancel returns in ~6ms with the late result discarded (not adopted), the app-owned deadline fires and discards, and a 300-iteration race stress test proving completion-versus-cancel never double-resumes the continuation.--strictclean. The GSS credentials file typechecks against the realGSS.framework.dbopenteardown runs through theCFreeTDSC bridge, soFreeTDSConnection/MSSQLPlugincompile only in the full Xcode build, and the end-to-end teardown needs a real (and for Windows Auth, Kerberos-secured) SQL Server to exercise.Files
New:
MSSQLConnectCancellation.swiftand its tests. Deleted:MSSQLKerberosConnectGate.swift. Changed:FreeTDSConnection.swift,MSSQLKerberosCredentials.swift,MSSQLPlugin.swift,MSSQLConnectionOptions.swift,MSSQLCoreError.swift, iOSMSSQLDriver.swift,CHANGELOG.md,CLAUDE.md. No pbxproj change (the deleted plugin file drops out of the filesystem-synchronized group; the new files live in the SwiftPMTableProMSSQLCoretarget).