Skip to content

Race condition in persistent sessions #739

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sb8244 opened this issue May 7, 2025 · 1 comment
Open

Race condition in persistent sessions #739

sb8244 opened this issue May 7, 2025 · 1 comment

Comments

@sb8244
Copy link

sb8244 commented May 7, 2025

I believe I have found a race condition bug with persistent sessions. I think it's pretty hard to recreate in a general reproduction. Although I can give it a shot if needed. The race is pretty visible to me now:

Condition

  • Expired session
  • valid persistent session
  • Easy way to consistently reproduce is to set the TTL to something very small on the session, like 3s, with a long persistent session TTL

Reproduce Bug

  • Have 2 tabs or more of the app loaded
  • Load across these tabs
  • Eventually some of the tabs will return a 401, with one of the tabs loading correctly
  • Reloading the 401 tabs causes them to load correctly (because the user isn't actually logged out)

Suspected Issue

PowPersistentSession.Plug.Cookie.before_send_delete/2 calls expire_token_in_store. This is running after a valid authorization (sometimes? not sure of when). If 2 requests start at the same time, and one of them completes before the other one tries to auth, then the session token will be invalid.

Maybe: This would be most likely to happen if the per-domain request limit is hit, so requests are sitting in a queueing state for longer than normal.

Solution

Working on figuring out a solution for my app right now. Not sure if it will be a wider solution or not.

@sb8244
Copy link
Author

sb8244 commented May 7, 2025

A thought that I have is that instead of instantly deleting the persistent session when it's updated, rather set the expiration to 60 seconds in future. The old session won't be usable after that point, but the requests will not fail.

Edit: Similar solution may also work:

  def delete(config, key) do
    if Keyword.get(config, :namespace) == "persistent_session" do
      Task.start(fn ->
        Process.sleep(10_000)
        Pow.Postgres.Store.delete(config, key)
      end)
    else
      Pow.Postgres.Store.delete(config, key)
    end
  end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant