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

GROOVY-8067: Possible deadlock when creating new ClassInfo entries in the cache #489

Closed
wants to merge 1 commit into from

Conversation

jwagenleitner
Copy link
Contributor

As suggested in PR #484 removed the locking on the ManagedLinkedList by creating a new ManagedConcurrentLinkedQueue.

Also added a stress subproject for tests that employ many threads, need GC, or just in general try to break things and take a long time. These require a special property to be set in order to run, otherwise they will be skipped. I tried to work it out in the performance subproject, but that seems to be very specialized for the compiler tests. Open to suggestions on a better way to handle these types of tests.

}
}
modifiedExpandos.add(this);
}
Copy link
Contributor Author

@jwagenleitner jwagenleitner Feb 5, 2017

Choose a reason for hiding this comment

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

Removed because it appeared to be a duplicate of the logic above. Only if the previous strongMetaClass value was an expando would it have been added to the modifiedExpandos, so the iteration/removal above should have taken care of removing this ClassInfo from the modifiedExpandos and there's no reason to iterate again.

@blackdrag
Copy link
Contributor

that looks very promising to me +1


}

private final class Iter implements Iterator<T> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

think this can be made a static nested class

Class<?> newClass = (cls == null) ? createRandomClass() : cls;
try {
startLatch.await();
} catch (InterruptedException ie) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

can use ThreadUtils.awaite()

@jwagenleitner
Copy link
Contributor Author

@blackdrag @paulk-asert do you have any reservations about adding a new sub-project for the manual/stress tests? I tried to work them into performance but it currently doesn't include the root project in the test dependencies. It also depends on Java 1.8 and I think it would be good to be able to run these tests in the min supported version 1.7.

@blackdrag
Copy link
Contributor

no reservations on my side

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.

2 participants