pallets-eco / flask-session

Server side session extension for Flask
https://flask-session.readthedocs.io
BSD 3-Clause "New" or "Revised" License
501 stars 239 forks source link

Race condition for two requests in close proximity #71

Closed sherzinger closed 6 months ago

sherzinger commented 7 years ago

I spent the whole morning debugging an issue where submitting two requests in close proximity will cause a race condition in the flask-session extension. I'm using the Redis session interface, but it might affect others as well.

Here is what happens:

  1. Submit two requests that will modify the session. E.g. session['my_list'].append('foo') and session['my_list'].append('bar')
  2. Due to the parallelisation of the requests both are executed at the same time
  3. open_session() is called twice before any of the two requests reaches save_session()
  4. The session value for both requests is now IDENTICAL. This is very bad.
  5. save_session() is called by one of the two, then the other. Since the retrieved values in step 4 are identical, one will overwrite the other.

This is roughly what happens in Redis:

GET session:123 -> [] GET session:123 -> [] SETEX session:123 [foo] SETEX session:123 [bar]

This is what should happen:

GET session:123 -> [] SETEX session:123 foo GET session:123 -> [foo] SETEX session:123 [foo, bar]

ThiefMaster commented 7 years ago

I think this is the case in pretty much every single webapp. I'd really avoid putting things in the session where this is a real problem.

Lxstr commented 6 months ago

@mephi42 has shown an interesting solution to this problems that covers most situations, however (in line with @ThiefMaster comment) I'm not convinced that this is a common enough issue for people to warrant significant changes and a new database structure that may not transition easily. I'd also be interested in the performance effects and concerned with the ease of administering when the data is so splintered (looking at sessions or cleaning up expired ones).

Also it would only solve the issue for SQLAlchemy backend. Closing this for now as not planned, but will reopen if there is enough interest.

It may be an opportunity for someone to create a high concurrency oriented fork and trailblaze.

Closing as not planned at this stage.