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

method annotated with @Transactional(isolation = Isolation.READ_COMMITTED) returns uncommitted data #2733

Open
jvanheesch opened this issue Dec 19, 2022 · 5 comments
Assignees
Labels
status: waiting-for-triage An issue we've not yet triaged

Comments

@jvanheesch
Copy link

Reproducer kan be found here: https://github.com/jvanheesch/spring-boot-isolation

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 19, 2022
@schauder
Copy link
Contributor

schauder commented Jan 2, 2023

Please simplify the reproducer, so it can be run locally without all that Docker overhead.

You shouldn't need a web application either.
A simple integration test should do.

@schauder schauder added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 2, 2023
@jvanheesch
Copy link
Author

I worked out an example without docker and web: https://github.com/jvanheesch/spring-boot-isolation/tree/SpringBootTest

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 2, 2023
@schauder
Copy link
Contributor

schauder commented Jan 3, 2023

The reproducer is broken.
In the first example you call Thread.start() https://github.com/jvanheesch/spring-boot-isolation/blob/SpringBootTest/src/test/java/io/github/jvanheesch/ExampleIT.java#L15

In the second example you call Thread.run() https://github.com/jvanheesch/spring-boot-isolation/blob/SpringBootTest/src/test/java/io/github/jvanheesch/ExampleIT.java#L26
This does NOT start a new thread, but runs the closure in the current thread. So everything else runs after the update is performed and committed.

The call to readUncommited is irrelevant.

You really shouldn't use Thread.sleep(millis) to weave your threads for such tests. Instead I recommend to use CountDownLatch

@schauder schauder closed this as not planned Won't fix, can't repro, duplicate, stale Jan 3, 2023
@schauder schauder added status: invalid An issue that we don't feel is valid and removed status: feedback-provided Feedback has been provided labels Jan 3, 2023
@jvanheesch
Copy link
Author

Using Thread.run() instead of Thread.start() was a sloppy and obvious mistake on my part. I've updated the reproducer and hope it can be reconsidered again.

@schauder
Copy link
Contributor

schauder commented Jan 3, 2023

Ok, that is strange.

I massaged the tests a little to use CountDownLatch for synchronization.
And also added a test to demonstrate the behaviour with just JDBC, which behaves as expected.

https://github.com/schauder/issue-jpa-2733-spring-boot-isolation/tree/SpringBootTest

@schauder schauder reopened this Jan 3, 2023
@mp911de mp911de added status: waiting-for-triage An issue we've not yet triaged and removed status: invalid An issue that we don't feel is valid labels Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

No branches or pull requests

5 participants