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

Fix TypeCache dead lock #902

Merged
merged 1 commit into from Jan 23, 2017
Merged

Fix TypeCache dead lock #902

merged 1 commit into from Jan 23, 2017

Conversation

raphw
Copy link
Member

@raphw raphw commented Jan 23, 2017

I could trace the problem to more eager resolution of types in Java u31 upon loading where concurrent mock creation with locking on the class level causes a dead lock. This can happen in other VM implementations but can be solved with a more granular lock on our type cache.

This fixes #892.

Copy link
Contributor

@TimvdLippe TimvdLippe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some unintended Android changes slipped in

@bric3
Copy link
Contributor

bric3 commented Jan 23, 2017

@raphw Could that be the issue that I saw in #897, see travis log : https://travis-ci.org/mockito/mockito/jobs/194254780#L360-L366

@raphw
Copy link
Member Author

raphw commented Jan 23, 2017

No, that is a flaky test as we rely on triggering a garbage collection which is something we do not controll on a VM even though there is a good chance to. We should ideally rerun this test multiple times upon a failure.

@TimvdLippe I fixed the accidental pre-commit.

@bric3
Copy link
Contributor

bric3 commented Jan 23, 2017

OK. Agreed.

@bric3 bric3 changed the title Dead lock Fix TypeCache dead lock Jan 23, 2017
@bric3 bric3 merged commit ec71d3b into release/2.x Jan 23, 2017
@bric3 bric3 deleted the dead-lock branch January 23, 2017 13:47
@bric3 bric3 removed the in progress label Jan 23, 2017
@@ -13,6 +13,8 @@

class TypeCachingBytecodeGenerator extends ReferenceQueue<ClassLoader> implements BytecodeGenerator {

private final Object BOOTSTRAP_LOCK = new Object();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this field intended to be constant? If not, it should have lower case name.

Copy link
Member Author

@raphw raphw Jan 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is true, this should be lower case but yes, it should be an instance constant.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However this lock should not be static.

Copy link
Member

@mockitoguy mockitoguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mockito 2.6.4 hangs on JDK 1.8.0_31
4 participants