Skip to content
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

fix session saving and loading data race #35

Closed
wants to merge 1 commit into from

Commits on Oct 9, 2023

  1. fix session saving and loading data race

    This fixes a bug where session data could be lost if a session is loaded
    from the store while it's also being saved back to the store. Because
    sessions are loaded and stored in memory at the beginning of a request,
    it was previously possible for a session's inner value to be correctly
    altered but for that modification to be lost between the moment the
    session is saved to the store and when it's loaded. In other words, the
    data loaded would not reflect the in-progress update and instead be
    stale.
    
    To address this we could hold a lock for the duration of the request.
    However, this imposes a performance penalty we would like to avoid.
    Alternatively we can manage a registry of sessions, a la tower-cookie's
    management of `CookieJar`: with this, we load a session first from the
    service's cache of sessions and fallback to the store only when the
    session is not present.
    
    Currently sessions are managed like this via DashMap. However,
    alternatively caching facilities, such as Moka, might prove to be a
    better fit. Further, the current implementation does not bound the total
    number of cached sessions--this should be addressed in future
    updates. At present, this means memory will continue to grow, albeit
    typically at a very small rate; this is a memory leak!
    
    Because sessions are safely written to and read from this cache, we
    prevent stale reads and thus a data race which could lead to data loss
    in highly concurrent environments, especially where multiple requests
    are made using the same session cookie simultaneously.
    
    Note that `replace_if_equal` is still required to prevent data races in
    application code.
    maxcountryman committed Oct 9, 2023
    Configuration menu
    Copy the full SHA
    e905333 View commit details
    Browse the repository at this point in the history