When there're mutiple concurrent requests using the same access_token (being an Authorization header of an access_token query parameter), the update of the last usage timestamp of the token leads to a database serialization issue. The exception bubbles and leads to the access being denied.
It happens on relatively moderate trafic (~10 simultaneous HTTP request using the same token).
If I comment out
// Track last access on the token.
$this->logAccessTime($token);
in OAuth2Storage::getAccessToken(), everything goes smoothly (albeit the timestamp not being updated, obviously).
I do not see any way to easily solve this.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | serialization_deadlock-2859214-5.patch | 869 bytes | sanduhrs |
| #4 | oauth2-deadlock-d7.patch | 739 bytes | lauriii |
| #2 | serialization_deadlock-2859214-2.patch | 635 bytes | garphy |
Comments
Comment #2
garphyThis patch is just a workaround to catch the Exception and avoid denying access to legitimate request.
This is obviously not a long-term solution.
Comment #3
garphy@maintainers : unfortunately, the proposed workaround doesn't work in all cases.
can you confirm that there indeed may be a transaction design issue here and hopefully give some guidance about how resolves this ?
i'm willing to spend some time to resolves this but I really don't know were to start
Comment #4
lauriiiHere's a patch for the Drupal 7 version for anyone running into this problem.
Comment #5
sanduhrsComment #7
sanduhrsComment #8
m.stentaWe ran into the same issue in farmOS, and the patch in #4 "fixes" it (with the exception that the token's
last_accessmight not be getting updated when it should in all cases).I did find this Drupal core issue that might be related: #1650930: Use READ COMMITTED by default for MySQL transactions
Setting this to RTBC because I think the patch in #4 can be committed to 7.x-1.x, to match the commit that was already made to 8.x-1.x. Not sure if you want to set it back to "Active" after that to consider how to proceed with the
@todoor not.Comment #10
cafuego commentedPatch from #4 is committed. Thank you!