Redis FIX: The same session id does not support concurrency

[0x1618]

The fix addresses the error causing the Flask-Session Redis integration to break. You can find more details about the issue in the following link: https://github.com/pallets-eco/flask-session/issues/149.

[0x1618]

@Lxstr Hey! It's been a while. You should look at this because it's a MUST-FIX. (The fix makes the Redis Interface usable)

[Lxstr]

Hi @0x1618 thanks for the ping. I note your PR uses

        # Check if the session has been modified
        if not session.modified:
            return

and

current_val = self.redis.get(self.key_prefix + session.sid)
        if current_val != val:
            # Update Redis store only if the session data has changed

Can you tell me how these two items work together? Is session.modified not being detected for Redis?

In Flask's session.py there is this note

    #: Some implementations can detect changes to the session and set
    #: this when that happens. The mixin default is hard coded to
    #: ``True``.
    modified = True

I currently have a development branch where I am working on an alternative path to merging all of the PRs. Perhaps you'd like to test out that one and see if there's any difference? It uses the should_set cookie method instead of only checking session.modified

[Lxstr]

Also, I'm having trouble repoducing the issue using the example.py in the project and using the latest release. I only get the session I'm expecting. Can you assist in reproducing?

[0x1618]

Can you tell me how these two items work together? Is session.modified not being detected for Redis?

I added these lines of code because I saw that in cases of concurrent requests or more complex functions, the old request containing new Redis data was being replaced by the next request containing the old Redis data (The person who reported the bug explained it well). It seemed peculiar, but it was happening. So, I included the condition if session.modified because the code should only run when the session has been modified. Later, I observed that this line of code sometimes caused issues, so I added a subsequent check using if new value != old value. Through this approach, I noticed that the concurrency problem disappeared.

Also, I'm having trouble repoducing the issue using the example.py in the project and using the latest release. I only get the session I'm expecting. Can you assist in reproducing?

To be honest with you, I don’t know exactly when this error occurs. For me, the error occurred only when I was sending requests to my login endpoint, other endpoints worked well. It’s a very weird bug, but when it occurs, it gives an instant headache. I can send screenshots when using the Flask session without the fix and with my fix.

[Lxstr]

Hi @0x1618

I've implemented a number of fixes for issues related to this one (possibly the same) on development branch. I would really appreciate if you are able to install that locally to test. It would also be good if I can see your login route (redacted or shortened if you need)

The modified check has been implemented. That should mean that only if you modify the session dictionary, there should be an update sent to the database. Note that according the the Flask documents, you may need to set session.modify = True if you are doing something to the session that may not be recognised as a change, for example adding to an array (mutation).

There has also been a change to the boolean evaluation of session to detect if empty based on @kgutwin input in #193. Previously this was not working when _permanent property was set, which is default in flask-session.

I am hoping that these two should do the trick without the need for your second element (which results in more db calls). But let's see..

[0x1618]

Hi @0x1618

I've implemented a number of fixes for issues related to this one (possibly the same) on development branch. I would really appreciate if you are able to install that locally to test. It would also be good if I can see your login route (redacted or shortened if you need)

The modified check has been implemented. That should mean that only if you modify the session dictionary, there should be an update sent to the database. Note that according the the Flask documents, you may need to set session.modify = True if you are doing something to the session that may not be recognised as a change, for example adding to an array (mutation).

There has also been a change to the boolean evaluation of session to detect if empty based on @kgutwin input in #193. Previously this was not working when _permanent property was set, which is default in flask-session.

I am hoping that these two should do the trick without the need for your second element (which results in more db calls). But let's see..

I tested the development branch, and it didn't work. The same issue persists. I can provide you with another endpoint that is quite simple, and the error occurs there frequently. I even have to send approximately four requests to make the endpoint save the data. Redis Data was saved only when I sent the fourth request. The 'Panel użytkownika' should appear after the first request, indicating that the data was saved. However, as you can see, the data was saved after the fourth request.

The code here is very simple. I believe that the issue is related to the multiprocessing/multithreaded Flask application running concurrently. Debugging this bug is challenging, and I would appreciate your trust to add my solution, as I can't even pinpoint where the bug originated.

[Lxstr]

Thanks for your quick response. I really want to try to pinpoint (or at least replicate) the origin of the issue before we make any changes, so we can ensure the most efficient solution and make sure the other database backends are not affected. Would you be able to help me by starting a repo where we try to reproduce endpoint with your same setup and deployment, etc? How are you running your flask application concurrently? Also, you're not using caching right?

It is expected that concurrent Redis setex commands on a key will only apply the last one, as I understand. But I would expect all of your commands from the endpoint should be the same right?

[Lxstr]

Hi @0x1618 I'm still interested in debugging this further, I just need more information about how you are running your flask app with what workers/threads etc.

[Lxstr]

Attempted to replace with uwsgi workers but seems to work fin for me. Closing due to no response and no reproduction.