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.

Sign up for GitHub

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

Jump to bottom

cursor / connection not cleaned up if result proxy or after cursor execute fails #5182

Closed
zzzeek opened this issue Mar 4, 2020 · 2 comments
Closed

cursor / connection not cleaned up if result proxy or after cursor execute fails #5182

zzzeek opened this issue Mar 4, 2020 · 2 comments
Labels
bug Something isn't working engine engines, connections, transactions, isolation levels, execution options
Milestone
1.3.x

Comments

@zzzeek
Copy link
Member

zzzeek commented Mar 4, 2020

on CI we are getting SQLite DB is locked due to test_dupe_col_obj which is doing connectionless execution, and due to some GC timing changes that seem to be a result of #4849, it's revealing the cursor and connection are not getting actively closed after the failure. the handle_dbapi_exception in Connection._execute_context() needs to be moved down to include the whole block.
@zzzeek zzzeek added bug Something isn't working engine engines, connections, transactions, isolation levels, execution options labels Mar 4, 2020
@zzzeek zzzeek added this to the 1.3.x milestone Mar 4, 2020
@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue in the master branch:

Run handle_error for any exceptions raised in execute_context() https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/1765

@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue in the rel_1_3 branch:

Run handle_error for any exceptions raised in execute_context() https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/1766

sqlalchemy-bot pushed a commit that referenced this issue Mar 4, 2020
@zzzeek
Run handle_error for any exceptions raised in execute_context()
47abc46 
Observing a SQLite connection/cursor being hung on
test_resultset -> PositionalTextTest -> test_dupe_col_obj.
this uses connectionless execution and the result object
fails to be constructed.  When that happens, there is no path
for the cursor or connection to be closed / released.
Recent changes with the exception assertions in #4849 seem to
be causing a cycle to last a little longer than usual which
is exposing this issue for one particular test on SQLite.

As we want to get rid of weakref cleanup, evaluate
why we dont have handle_dbapi_exception for this whole
block, as after_cursor_execute can raise, result construction
can raise, autocommit can raise, close can raise, there
does not seem to be a reason these things should be outside
of the block that gets cleaned up.

Fixes: #5182
Change-Id: I640ac55e8c5f39d287f779fbb5dc0ab727218ca3
(cherry picked from commit 4cd33f18a21d9e33b37ef7163822c327453d1e62)
@alvassin alvassin mentioned this issue Apr 11, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working engine engines, connections, transactions, isolation levels, execution options
Projects
None yet
Milestone
1.3.x
Development

No branches or pull requests
2 participants
@zzzeek @sqla-tester