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

[PAYARA-3860] - Added synchronization block double check to avoid race condition #4021

Merged
merged 5 commits into from Jun 11, 2019

Conversation

@cubastanley
Copy link
Contributor

commented Jun 7, 2019

Added a synchronized double check in PayaraInstanceImpl to avoid race condition - tested through sheer brute forcing of Thread.sleep() and spamming of functionality and problem no longer appears to be present

@cubastanley cubastanley changed the title Added synchronization block double check to avoid race condition [PAYARA-3860] - Added synchronization block double check to avoid race condition Jun 7, 2019

@cubastanley

This comment has been minimized.

Copy link
Contributor Author

commented Jun 7, 2019

Jenkins test please

@pdudits
pdudits approved these changes Jun 7, 2019
if (me == null) {//race check
initialiseInstanceDescriptor();
//Race condition double check
if (me == null) {

This comment has been minimized.

Copy link
@pdudits

pdudits Jun 7, 2019

Contributor

Oh, why did I miss that article? :)

Likely the easiest solution then is to replace any access to me with synchronized getter that can also initialize the descriptor when null.

This comment has been minimized.

Copy link
@cubastanley

cubastanley Jun 7, 2019

Author Contributor

Is something like

private synchronized InstanceDescriptorImpl getMyInstanceDescriptor() {
        if(me == null) {
            initialiseInstanceDescriptor();
        }
        return me;
    }

the kind of thing you mean?

@cubastanley cubastanley requested review from Pandrex247 and pdudits Jun 7, 2019

@cubastanley

This comment has been minimized.

Copy link
Contributor Author

commented Jun 7, 2019

Jenkins test please

@Inject
private PayaraExecutorService executor;

private synchronized InstanceDescriptorImpl getMyInstanceDescriptor() {
if(me == null) {

This comment has been minimized.

Copy link
@pdudits

pdudits Jun 10, 2019

Contributor

space after if, please

@@ -239,50 +243,46 @@ void postConstruct() {
@SuppressWarnings({"unchecked"})
public void event(Event event) {
if (event.is(EventTypes.SERVER_READY)) {
initialiseInstanceDescriptor();
PayaraInternalEvent pie = new PayaraInternalEvent(PayaraInternalEvent.MESSAGE.ADDED, me);
//initialiseInstanceDescriptor();

This comment has been minimized.

Copy link
@pdudits

pdudits Jun 10, 2019

Contributor

remove this one as well

@cubastanley

This comment has been minimized.

Copy link
Contributor Author

commented Jun 10, 2019

Jenkins Test Please

@cubastanley cubastanley merged commit 4ee9895 into payara:master Jun 11, 2019

58 of 59 checks passed

security/snyk - appserver/ant-tasks/pom.xml (payara-ci) 4 new vulnerable dependency paths
Details
Payara Quick Build and Test Quick build and test passed!
Details
security/snyk - api/payara-api/pom.xml (payara-ci) No new issues
Details
security/snyk - api/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admingui/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/appclient/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/batch/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/common/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/concurrent/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/connectors/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/core/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ejb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/extras/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/featuresets/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ha/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/installer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/javaee-api/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jdbc/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/load-balancer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/orb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/payara-appserver-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/persistence/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/registration/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/security/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/transaction/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/web/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/webservices/pom.xml (payara-ci) No new issues
Details
security/snyk - copyright/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/cluster/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/common/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/core/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/diagnostics/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/hk2/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/payara-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources-l10n/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/security/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/test-utils/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - pom.xml (payara-ci) No new issues
Details

@arjantijms arjantijms added this to the 5.193 milestone Jul 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.