flask-sqlalchemy icon indicating copy to clipboard operation
flask-sqlalchemy copied to clipboard

session.remove called too early when there are stacked application contexts

Open arikfr opened this issue 7 years ago • 2 comments

Currently flask-sqlalchemy will call session.remove() when app.teardown_appcontext is called. Because application contexts can be stacked, it means that we will remove the session when the inner most application context is popped, while there still might be other active contexts.

Aren't sessions supposed to be scoped per application context?

Here's an example to show the issue:

from flask import Flask
from flask_sqlalchemy import SQLAlchemy

app = Flask(__name__)
app.config['SQLALCHEMY_DATABASE_URI'] = 'sqlite://'
db = SQLAlchemy()
db.init_app(app)


class User(db.Model):
    id = db.Column(db.Integer, primary_key=True)
    name = db.Column(db.String())


def test_session_remove_example():
    with app.app_context():
        db.create_all()

        user = User(name='a')
        db.session.add(user)
        assert user in db.session

        with app.app_context():
            assert user in db.session

        # This will fail, because the session was removed.
        assert user in db.session

arikfr avatar Jun 22 '17 07:06 arikfr

out of curiosity, why do you have multiple instances of the same app's context on the stack?

ThiefMaster avatar Jun 22 '17 07:06 ThiefMaster

Good question :) In my background jobs, before using Flask-SQLAlchemy, I didn't need application context for all tasks, so I was explicitly pushing it for a few of the tasks that did need it (to use Flask-Mail).

After switching to Flask-SQLAlchemy, I changed the background jobs base class to push application context before running the tasks as it was always required now. This resulted in the application context being pushed twice and exposing this issue.

I can't imagine a real world scenario when this might happen, but because it does expose some unexpected behavior, I thought it's worth reporting.

arikfr avatar Jun 22 '17 19:06 arikfr

#1087 changes the session scope to use the current app context instead of the thread. If there are different contexts pushed, there is a different session for each context.

davidism avatar Sep 18 '22 17:09 davidism