Problem/Motivation
When multiple processes request the system access token at the same time and the token is expired, they contend for the refresh lock in TokenManager::refreshSystemAccessToken(). A process that fails to
acquire the lock waits, then re-reads the token from state hoping another process has already refreshed it.
However, the re-read uses the wrong state key: it reads $this->lockName (the lock's key, e.g. <prefix>..system_token:refresh) instead of $this->systemTokenName (the key the token is actually stored under).
As a result the freshly refreshed token is never found, the wait loop does not short-circuit, and the process falls through to performing its own redundant token refresh against the OAuth2 provider once it finally acquires the lock.
This defeats the purpose of the lock — it produces extra, unnecessary refresh requests to the provider under concurrency and can churn the refresh token.
Steps to reproduce
- Have an expired system access token stored in state.
- Trigger two or more concurrent requests that call
getSystemAccessToken(). - One request acquires the lock and refreshes the token; the others wait.
- Observe that the waiting requests do not pick up the refreshed token and each perform their own refresh against the provider.
Proposed resolution
In the wait loop of refreshSystemAccessToken(), read the token from state using $this->systemTokenName instead of $this->lockName, so a token refreshed by another process is detected and returned without a redundant refresh.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | oauth2_token_manager-lock_contention_reread-3624923-2.patch | 6.26 KB | peximo |
Issue fork oauth2_token_manager-3624923
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
peximo commentedAttached patch implements the proposed fix and adds tests
Comment #5
heddn