fix(datastore): stop a stale lock locking a player out permanently

[buildthomas]

Two defects that compose into players being unable to join at all, indefinitely, with no recovery short of editing their key by hand. Confirmed in production; stripping the lock entry from an affected key over Open Cloud fixes that player every time, which is what localised it.

The kick: lock age decides one question and not the other

AcquireLock, on load, treats a foreign lock older than GetAutoSaveTimeSeconds() * UNLOCK_BY_DEFAULT_TIME_MULTIPLIER as belonging to a crashed server and takes it. ToUnlockedProfile, guarding the save, only ever asks whether the lock is ours — LastUpdateTime appears nowhere in it. So a foreign lock that outlives a load is stealable on load and fatal on save, at any age: the save fires SessionStolen, PlayerDataStoreManager kicks with "DataStore session stolen by another active session", and the write that was about to land is dropped. Waiting does not help — the save path has no notion of age to wait out. We had players locked out 11+ hours, kicked within two seconds of every join.

Both halves now ask _isLockStale, so they cannot disagree about the same lock. The save that follows rewrites the lock as ours, so an affected key heals itself.

The rule loosened is a save by a session whose lock was taken after going quiet past the protocol's own definition of dead. A fresh foreign lock still reports theft, and one with no LastUpdateTime still does — both covered by tests. The alternative reading is that the save side should stay strict and the load side should guarantee it never leaves a foreign lock behind; I went with symmetry because any other route to a surviving foreign lock reproduces the lockout, and two halves disagreeing about the same predicate is a defect regardless of the threshold chosen.

The persistence: a teardown that lands mid-request aborts the write

Transform function error ...datastore.Server.DataStore:696:
attempt to call missing method 'AcquireLock' of table

Promise.spawn hands the request to a thread it does not retain, so the promise a teardown cancels is not the call — Roblox invokes the transform regardless. By then BaseObject.Destroy has run setmetatable(obj, nil) on the store and on its session-locking helper, so the transform's first method dispatch raises, and the raise aborts the write Roblox was about to commit. On the save path (line 591, ToUnlockedProfile) that silently drops a player's staged data. On the load path it kills the steal-write that would have replaced a stale foreign lock — invisibly, because the load already resolved from inside the transform, and the late failure lands in a :Catch guarded by IsPending(), which is already false.

Because a metatable-less table still serves field reads but no method calls, the guard cannot itself be a method on the store — post-Destroy, self:anything() is the same class of crash. Both transforms read self._sessionLockingEnabledHelper as a raw field and treat a helper whose metatable is gone as proof of teardown, cancelling through the transforms' existing cancel path. Stores without session locking keep their old behavior (the save transform's IsRejected check cancels them).

How the loop closed in production

1. A server dies holding the lock.
2. Rejoin: the load steals the stale lock and resolves in-memory, inside the transform, before the commit.
3. The consuming game saves within a second or two of join (join-time reconciler services); that save still sees the foreign lock → SessionStolen → kick.
4. The kick's removal drops the close-write on the same theft check, then destroys the store while the steal-write can still be in flight, aborting it. The failure is swallowed.
5. Key unchanged; every join replays.

Commit 1 removes step 3 (and step 4's theft check); commit 2 removes step 4's abort. The exact interleaving in step 3/4 depends on per-key request scheduling that source alone can't prove, but it is the only mechanism we found consistent with a key still holding an 11-hour-old lock after many join attempts.

Not fixed here, flagged as follow-ups

• The load resolves from inside its transform, before the write commits, and a late failure of that write is discarded by the IsPending() guard — success, throttle, and abort are indistinguishable to the caller. That blindness enabled the loop; resolving after the commit settles is a behavioural change I did not want to fold in uninvited.
• A save dropped by the theft check returns nil from the transform, which is a successful no-op UpdateAsync — Save()/SaveAndCloseSession() resolve, and callers (receipt processors, shutdown flushes) believe data persisted that did not.

Also included

• IsLoadPending(), and a traceback log in _removePlayerDataStore when a store is removed with its first load still outstanding — the window the teardown bug lives in. Gated on that window rather than logged per removal, which would be one traceback per player per shutdown.
• DataStoreMock records a throwing transform (GetLastTransformError); without it a spec cannot tell an aborted write from a cancelled one, since both leave the store untouched and the pcall in DataStorePromises swallows the error.

Tests

• DataStore.SessionLock.spec.lua: a stale foreign lock validates on the save side as it does on the load side; slightly-old and LastUpdateTime-less foreign locks still report theft.
• DataStore.TeardownDuringRequest.spec.lua (new): destroy mid-load, destroy mid-save, and no lock left behind afterwards. Each blocks the mock, starts the request, destroys the store, unblocks, then asserts the transform did not raise and nothing was written. The mock parks blocked requests in a task.wait loop, so the resumed thread genuinely runs the transform against an already-destroyed store — the production scenario.

Verification

stylua and selene clean; the specs were hand-traced against the mock's blocking semantics. The local lint:luau/test toolchain is broken on this machine (rojo --version panics on the aftman spec), and no CI checks have run on this fork PR yet — the suite needs a CI run before this merges.