(fix) Change context manager type to avoid race in threads#1368
Conversation
…oid thread race at the cost of working with the generator
b881e22 to
a2375b0
Compare
|
@Fofanko if this really fixes the problem, happy to merge it in, but I'm trying to fully understand where the threading problem actually lies. In the original stacktrace on the linked issue, the error occurs in From the
So I believe the actual fix for this would be to wrap the |
sl0thentr0py
left a comment
sl0thentr0py
left a comment
There was a problem hiding this comment.
thx @Fofanko for the PR!
I'm still not convinced this is the actual problem, but it still does get rid of generator magic so I'm merging it in.
|
fyi @Fofanko I'm reverting this since it caused a regression and trying to fix the problem properly in #1388. |
…gration (#1388) * Revert "fix(sqlalchemy): Change context manager type to avoid race in threads (#1368)" This reverts commit de0bc50. This caused a regression (#1385) since the span finishes immediately in __enter__ and so all db spans have wrong time durations. * Use context instead of conn in sqlalchemy hooks
|
Can you add something like record_sql_queries = RecordSqlQueriesto the module? This breaks users who were using the previous name. |
|
@Forty-Bot this is reverted anyway in one of the newer releases. |
|
Ah, looks like I was looking at an older branch. |
3 participants
Fix issue from issue #1132 when we get race condition in multithreading with generator based context manager. The generator is not thread safe!