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.

Sign up for GitHub

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

Jump to bottom

PAYARA-3487 Race condition in ConnectionPool #3924

Merged
merged 2 commits into from May 14, 2019
Merged

PAYARA-3487 Race condition in ConnectionPool #3924

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6cac65a
When a connectionpool is closed it now logs an error rather than thro…
Cousjava Jan 22, 2019
f1cbfc0
PAYARA-3487 only log stacktrace if logger level set to FINE
Cousjava May 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 28 additions & 0 deletions ...onnectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/ResourceState.java
View file
Open in desktop
Expand Up @@ -37,13 +37,23 @@
* only if the new code is made subject to such option by the copyright
* holder.
*/
// Portions Copyright [2019] Payara Foundation and/or affiliates

package com.sun.enterprise.resource;

import com.sun.enterprise.resource.pool.ConnectionPool;
import com.sun.logging.LogDomains;
import java.util.logging.Level;
import java.util.logging.Logger;

public class ResourceState {
private boolean enlisted;
private boolean busy;
private long timestamp;
private TwiceBusyException busyException;
jbee marked this conversation as resolved.
Show resolved Hide resolved

//This is the same logger as in ConnectionPool, used to check the log level
private Logger LOGGER = LogDomains.getLogger(ConnectionPool.class,LogDomains.RSR_LOGGER);

public boolean isEnlisted() {
return enlisted;
Expand All @@ -67,6 +77,20 @@ public boolean isBusy() {

public void setBusy(boolean busy) {
this.busy = busy;
if (!busy && LOGGER.isLoggable(Level.FINE)) {
busyException = new TwiceBusyException();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be controlled by a system property? I believe there is significant overhead in collecting stacktrace at every connection close.

}
}

/**
* Gets an exception with a stack trace of when the resource was previously
* set to not busy.
* @return a TwiceBusyException used to create a MultiException when setBusy
* is set to false twice
* @see com.sun.enterprise.resource.pool.ConnectionPool#resourceClosed(com.sun.enterprise.resource.ResourceHandle)
*/
public TwiceBusyException getBusyStackException(){
return busyException;
}

public long getTimestamp() {
Expand All @@ -81,7 +105,11 @@ public ResourceState() {
touchTimestamp();
}

@Override
public String toString() {
return "Enlisted :" + enlisted + " Busy :" + busy;
}

public class TwiceBusyException extends Exception {}

}
12 changes: 10 additions & 2 deletions ...ors/connectors-runtime/src/main/java/com/sun/enterprise/resource/pool/ConnectionPool.java
View file
Open in desktop
Expand Up @@ -71,6 +71,7 @@
import java.util.logging.Logger;
import org.glassfish.api.admin.ServerEnvironment;
import org.glassfish.internal.api.Globals;
import org.glassfish.hk2.api.MultiException;

/**
* Connection Pool for Connector & JDBC resources<br>
Expand Down Expand Up @@ -1005,6 +1006,7 @@ public void deleteResource(ResourceHandle resourceHandle) {
* this method is called to indicate that the resource is
* not used by a bean/application anymore
*/
@Override
public void resourceClosed(ResourceHandle h)
throws IllegalStateException {
if (_logger.isLoggable(Level.FINE)) {
Expand All @@ -1017,9 +1019,15 @@ public void resourceClosed(ResourceHandle h)
}

if (!state.isBusy()) {
throw new IllegalStateException("state.isBusy() : false");
//throw new IllegalStateException("state.isBusy() : false");
_logger.log(Level.WARNING, "state.isBusy already set to false for {0}#{1}", new Object[]{h.getName(), h.getId()});
if (_logger.isLoggable(Level.FINE)) {
MultiException noBusyException = new MultiException(state.getBusyStackException());
noBusyException.addError(new IllegalStateException("state.isBusy() : false"));
_logger.log(Level.WARNING, null, noBusyException);
}
}

setResourceStateToFree(h); // mark as not busy
state.touchTimestamp();

Expand Down