2015-09-14 12:47 GMT+03:00 <ma...@apache.org>: > Author: markt > Date: Mon Sep 14 09:47:50 2015 > New Revision: 1702887 > > URL: http://svn.apache.org/r1702887 > Log: > Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=58370 > Fix data races when executor is being shut down > > Modified: > tomcat/tc8.0.x/trunk/ (props changed) > tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java > tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml >
> Modified: > tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java > URL: > http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java?rev=1702887&r1=1702886&r2=1702887&view=diff > ============================================================================== > --- > tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java > (original) > +++ > tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java > Mon Sep 14 09:47:50 2015 > @@ -576,8 +581,10 @@ public abstract class AbstractEndpoint<S > } > > public void shutdownExecutor() { > - if ( executor!=null && internalExecutor ) { > - if ( executor instanceof ThreadPoolExecutor ) { > + Executor executor = this.executor; > + this.executor = null; > + if (executor != null && internalExecutor) { > + if (executor instanceof ThreadPoolExecutor) { > //this is our internal one, so we need to shut it down > ThreadPoolExecutor tpe = (ThreadPoolExecutor) executor; > tpe.shutdownNow(); > @@ -595,7 +602,6 @@ public abstract class AbstractEndpoint<S > TaskQueue queue = (TaskQueue) tpe.getQueue(); > queue.setParent(null); > } > - executor = null; > } > } > In the old implementation of shutdownExecutor() the "executor = null;" assignment is conditioned by internalExecutor flag. In the new implementation it is assigned unconditionally. I think this change in behaviour is wrong. If it is an external executor (internalExecutor flag is false), this reference should not be nulled. shutdownExecutor() is called from stopInternal() of NioEndpoint etc., so in theory it is possible to call start() on endpoint if it has not been destroy()ed yet. Nulling the reference changes behaviour of such start(). Best regards, Konstantin Kolinko --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
