[avsej]

Fixes PCBC-1057.

Problem

Cluster::setAuthenticator() changes the credentials of the cached connection, but the connection stays under the key derived from the previous credentials (Cluster.php#L353-L356):

step	result
connect with credentials A	connection cached under key(A)
setAuthenticator(B)	new sockets authenticate as B; sockets already open stay authenticated as A
connect again with A	gets that connection, so requests may run as B; no authentication error for A

Depends on #267.

Change

• The connection leaves the cache. After the credentials are applied, the entry moves to a key no lookup produces and is marked detached. The cluster that called setAuthenticator() keeps using it. Clusters connected later bootstrap their own connection, whichever credentials they carry. The ticket's option A instead re-registers the connection under key(B). That would hand a cluster connecting with B sockets still authenticated as A, even when B is invalid or less privileged.

• Detaching does not free the resource. Deleting a persistent_list bucket runs plist_entry_destructor, which frees the zend_resource while PHP values still reference it. The destructor is suspended around the delete:

  auto* destructor = EG(persistent_list).pDestructor;
  EG(persistent_list).pDestructor = nullptr;   // <- res stays allocated
  zend_hash_del_bucket(&EG(persistent_list), bucket);
  EG(persistent_list).pDestructor = destructor;

• Detached connections are freed after the request releases its variables. The sweep runs in the module's post-deactivate hook, after PHP has destroyed request globals, so a detached cluster kept in a global array is freed at the end of that request. It does so whatever couchbase.persistent_timeout and couchbase.max_persistent are. The expiry pass is evaluated after that sweep, and walks the persistent list only when those settings enable expiry.

• Unreferenced detached connections are also freed during the request. Releasing the last PHP value of a persistent resource does not destroy it. Creating or detaching a connection therefore sweeps detached entries whose GC_REFCOUNT is 0, so a long-running CLI request does not keep one connection open per setAuthenticator() call. Detached connections are kept in a per-thread list, so the sweep visits only them and cache hits do not walk the persistent list.

• phpinfo() reports couchbase_persistent_connections, the number of open persistent connections.

• Transactions and ScanResults hold the connection resource. They run on the connection's core cluster, but held no PHP reference to it, so the sweep could close the cluster under a live transaction or scan. Their native objects are destroyed on separate threads, so each also holds a token from the connection handle. The sweep skips a handle whose token is still held.

• testSetAuthenticatorRemovesConnectionFromCache sets invalid credentials on a cached cluster. It checks that a cluster connected afterwards gets a different connection, and that connecting with the invalid credentials fails with AuthenticationFailureException instead of reusing the detached one.

• testSetAuthenticatorDoesNotAccumulateDetachedConnections repeats connect, setAuthenticator() and release, and checks that the connection count stays flat.

• testSweepKeepsDetachedConnectionUsedByTransactions and testSweepKeepsDetachedConnectionUsedByScan each release the cluster after setAuthenticator(), force a sweep, and then run a transaction or a scan created earlier.

• testSetAuthenticatorToCredentialsOfAnotherCachedConnection switches a cluster to the credentials of a second cached connection. The second connection stays the one shared under those credentials, and the switched cluster keeps serving KV operations on its own.

[Copilot]

🟡 Changes recommended

Rekeying a cached handle can expose sockets authenticated as the previous user, and mutable options can also cause connections to be reused with the wrong configuration.

4 open issues

• Reusing the handle can retain the previous user's authentication · New
• Derive the new key from the connection's original options · New
• Add persistent-worker coverage for detached-resource cleanup · New
• Verify the moved connection has a healthy endpoint · New

What changed in this PR

This PR addresses PCBC-1057 by moving cached connections to a new credential-derived key when Cluster::setAuthenticator() is called, building on the full-options cache key introduced in #267.

Changes:

• Derives cache keys from all exported options and recalculates them when credentials change.
• Adds resource-preserving cache moves, collision handling, and shutdown cleanup for detached connections.
• Adds regression tests for option-based sharing, credential changes, and occupied destination keys.

File	Description
tests/​ConnectionCacheTest.php	Tests cache sharing, credential re-keying, and collisions.
src/​wrapper/​persistent_connections_cache.hxx	Declares cache movement and updates the cleanup callback signature.
src/​wrapper/​persistent_connections_cache.cxx	Implements resource-preserving moves and detached-entry cleanup.
src/​wrapper/​connection_handle.hxx	Tracks detached state and allows updating the connection hash.
src/​php_couchbase.cxx	Passes new cache keys to native code and always runs the shutdown sweep.
Couchbase/​ClusterOptions.php	Removes the authenticator-only hash helper.
Couchbase/​Cluster.php	Computes full-options cache keys and requests re-keying after authenticator changes.

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

Re-keying can expose sockets authenticated as a privileged prior user to callers with new credentials, and mutable options can cause connections to be reused with the wrong configuration.

6 open issues

• Keep old-authenticated connections out of the new credential cache · New
• Reusing the handle can retain the previous user's authentication
• Clean up the user when propagation wait times out · New
• Verify the moved connection has a healthy endpoint
• Add persistent-worker coverage for detached-resource cleanup
• Derive the new key from the connection's original options

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

Detached connections and sockets can remain allocated in long-running workers or escape request-shutdown cleanup while still referenced.

3 open issues

• Detached connections leak until shutdown or a later sweep · New
• Add persistent-worker coverage for detached-resource cleanup
• Invalid-credential test does not cover cached connections · New

5 resolved since last review

• Keep old-authenticated connections out of the new credential cache
• Reusing the handle can retain the previous user's authentication
• Clean up the user when propagation wait times out
• Verify the moved connection has a healthy endpoint
• Derive the new key from the connection's original options

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

The sweep can close connections still used by live dependent objects, while also missing some request-end cleanup and slowing cache lookups.

3 open issues

• Dependent objects can outlive the resource and lose their cluster · New
• Connection lookup sweeps the entire resource table on every cache hit · New
• Add persistent-worker coverage for detached-resource cleanup

2 resolved since last review

• Detached connections leak until shutdown or a later sweep
• Invalid-credential test does not cover cached connections

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

Detached connections can leave sockets open after request shutdown, while cleanup scans of the full persistent resource list can degrade performance.

3 open issues

• Skip shutdown sweeps when expiry and detached cleanup are disabled · New
• Add persistent-worker coverage for detached-resource cleanup
• Test Transactions and ScanResults lifetime guarantees independently · New

2 resolved since last review

• Dependent objects can outlive the resource and lose their cluster
• Connection lookup sweeps the entire resource table on every cache hit

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Detached connections can cause full persistent-list scans on every lookup and remain open after request shutdown when held in globals.

1 open issue

• Add persistent-worker coverage for detached-resource cleanup

2 resolved since last review

• Skip shutdown sweeps when expiry and detached cleanup are disabled
• Test Transactions and ScanResults lifetime guarantees independently

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

Detached connections can remain open after request shutdown, while referenced detached connections cause every cache lookup to scan the entire persistent list.

2 open issues

• Avoid scanning the full persistent list on every cluster lookup · New
• Add persistent-worker coverage for detached-resource cleanup

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

The shutdown sweep can leave detached connections open after request globals are released, while computing expiry before the sweep can unnecessarily evict reusable cached connections.

3 open issues

• Shutdown sweep misses clusters retained in request globals · New
• Compute expiry after sweeping detached connections · New
• Add persistent-worker coverage for detached-resource cleanup

1 resolved since last review

• Avoid scanning the full persistent list on every cluster lookup

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

Detached teardown can leave idle connections open indefinitely, and no multi-request regression test verifies post-deactivation cleanup.

3 open issues

• Add multi-request test for reclaiming global-held connections · New
• Schedule cleanup after detached dependency teardown · New
• Add persistent-worker coverage for detached-resource cleanup

2 resolved since last review

• Compute expiry after sweeping detached connections
• Shutdown sweep misses clusters retained in request globals

_{Review effort: Balanced}