Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Issue #3361 thread set setHandlers 

protect handler mutation with synchronized.
Correctly use copy-on-write semantics for accessing volatile

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2019-02-20 14:15:51 +11:00
parent 5b959f17af
commit b478332793
2 changed files with 62 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandlerCollection.java
View File

@ -155,9 +155,12 @@ public class ContextHandlerCollection extends HandlerCollection
@Override
public void setHandlers(Handler[] handlers)
{
super.setHandlers(handlers);
if (isStarted())
mapContexts();
synchronized (this)
{
super.setHandlers(handlers);
if (isStarted())
mapContexts();
}
}
/* ------------------------------------------------------------ */
@ -176,10 +179,6 @@ public class ContextHandlerCollection extends HandlerCollection
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
Handler[] handlers = getHandlers();
if (handlers==null || handlers.length==0)
return;
HttpChannelState async = baseRequest.getHttpChannelState();
if (async.isAsync())
{
@ -196,19 +195,19 @@ public class ContextHandlerCollection extends HandlerCollection
}
}
// data structure which maps a request to a context; first-best match wins
// { context path => [ context ] }
// }
if (target.startsWith("/"))
{
Trie<Map.Entry<String,Branch[]>> pathBranches = _pathBranches;
if (pathBranches==null)
return;
int limit = target.length()-1;
while (limit>=0)
{
// Get best match
Map.Entry<String,Branch[]> branches = _pathBranches.getBest(target,1,limit);
Map.Entry<String,Branch[]> branches = pathBranches.getBest(target,1,limit);
if (branches==null)
break;
@ -228,7 +227,9 @@ public class ContextHandlerCollection extends HandlerCollection
}
else
{
// This may not work in all circumstances... but then I think it should never be called
Handler[] handlers = getHandlers();
if (handlers==null)
return;
for (int i=0;i<handlers.length;i++)
{
handlers[i].handle(target,baseRequest, request, response);

78
jetty-server/src/main/java/org/eclipse/jetty/server/handler/HandlerCollection.java
View File

@ -86,44 +86,48 @@ public class HandlerCollection extends AbstractHandlerContainer
*/
public void setHandlers(Handler[] handlers)
{
if (!_mutableWhenRunning && isStarted())
throw new IllegalStateException(STARTED);
if (handlers!=null)
synchronized (this)
{
// check for loops
for (Handler handler:handlers)
if (handler == this || (handler instanceof HandlerContainer &&
Arrays.asList(((HandlerContainer)handler).getChildHandlers()).contains(this)))
throw new IllegalStateException("setHandler loop");
// Set server
for (Handler handler:handlers)
if (handler.getServer()!=getServer())
handler.setServer(getServer());
if (!_mutableWhenRunning && isStarted())
throw new IllegalStateException(STARTED);
if (handlers!=null)
{
// check for loops
for (Handler handler:handlers)
if (handler == this || (handler instanceof HandlerContainer &&
Arrays.asList(((HandlerContainer)handler).getChildHandlers()).contains(this)))
throw new IllegalStateException("setHandler loop");
// Set server
for (Handler handler:handlers)
if (handler.getServer()!=getServer())
handler.setServer(getServer());
}
Handler[] old=_handlers;
_handlers = handlers;
updateBeans(old, handlers);
}
Handler[] old=_handlers;;
_handlers = handlers;
updateBeans(old, handlers);
}
/* ------------------------------------------------------------ */
/**
* @see Handler#handle(String, Request, HttpServletRequest, HttpServletResponse)
*/
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response)
throws IOException, ServletException
{
if (_handlers!=null && isStarted())
if (isStarted())
{
Handler[] handlers = _handlers;
if (handlers==null || handlers.length==0)
return;
MultiException mex=null;
for (int i=0;i<_handlers.length;i++)
for (int i=0;i<handlers.length;i++)
{
try
{
_handlers[i].handle(target,baseRequest, request, response);
handlers[i].handle(target,baseRequest, request, response);
}
catch(IOException e)
{
@ -147,7 +151,6 @@ public class HandlerCollection extends AbstractHandlerContainer
else
throw new ServletException(mex);
}
}
}
@ -158,7 +161,10 @@ public class HandlerCollection extends AbstractHandlerContainer
*/
public void addHandler(Handler handler)
{
setHandlers(ArrayUtil.addToArray(getHandlers(), handler, Handler.class));
synchronized(this)
{
setHandlers(ArrayUtil.addToArray(getHandlers(), handler, Handler.class));
}
}
/* ------------------------------------------------------------ */
@ -168,16 +174,22 @@ public class HandlerCollection extends AbstractHandlerContainer
*/
public void prependHandler(Handler handler)
{
setHandlers(ArrayUtil.prependToArray(handler, getHandlers(), Handler.class));
synchronized (this)
{
setHandlers(ArrayUtil.prependToArray(handler, getHandlers(), Handler.class));
}
}
/* ------------------------------------------------------------ */
public void removeHandler(Handler handler)
{
Handler[] handlers = getHandlers();
synchronized (this)
{
Handler[] handlers = getHandlers();
if (handlers!=null && handlers.length>0 )
setHandlers(ArrayUtil.removeFromArray(handlers, handler));
if (handlers!=null && handlers.length>0 )
setHandlers(ArrayUtil.removeFromArray(handlers, handler));
}
}
/* ------------------------------------------------------------ */
@ -196,8 +208,12 @@ public class HandlerCollection extends AbstractHandlerContainer
{
if (!isStopped())
throw new IllegalStateException("!STOPPED");
Handler[] children=getChildHandlers();
setHandlers(null);
Handler[] children;
synchronized (this)
{
children = getChildHandlers();
setHandlers(null);
}
for (Handler child: children)
child.destroy();
super.destroy();