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.

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

Execute unprepared statements atomically in system tests #36859

Closed
wants to merge 1 commit into from

Conversation

97jaz
Copy link
Contributor

@97jaz 97jaz commented Aug 5, 2019

In system tests, a single database connection is shared among all
the server threads. A call to unprepared_statement temporarily
modifies an instance variable on the connection object, which is
then visible to other concurrently running threads. This leads to
a situation where prepared statements may end up with the wrong
binds.

Addresses #36763

In system tests, a single database connection is shared among all
the server threads. A call to unprepared_statement temporarily
modifies an instance variable on the connection object, which is
then visible to other concurrently running threads. This leads to
a situation where prepared statements may end up with the wrong
binds.

Addresses rails#36763
@97jaz
Copy link
Contributor Author

97jaz commented Aug 5, 2019

Oh, I should mention that this patch does fix the problem in https://github.com/careport/racy.
Also, the build failure appears unrelated.

@matthewd
Copy link
Member

matthewd commented Aug 6, 2019

Sounds fair to me.

No need for using_lock_thread? -- we can just always lock, as the other methods do.

@97jaz
Copy link
Contributor Author

97jaz commented Aug 6, 2019

Actually, this does not consistently fix the problem in #36736. Which isn't surprising, since the non-unprepared_statement code paths aren't protected by locks.

@97jaz 97jaz closed this Aug 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants