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

Issue #97 ServletHolder unavailable handling #4083

Merged
merged 2 commits into from
Sep 13, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ public abstract class BaseHolder<T> extends AbstractLifeCycle implements Dumpabl
{
private static final Logger LOG = Log.getLogger(BaseHolder.class);

protected final Source _source;
protected transient Class<? extends T> _class;
protected String _className;
protected boolean _extInstance;
protected ServletHandler _servletHandler;
private final Source _source;
private Class<? extends T> _class;
private String _className;
private T _instance;
private ServletHandler _servletHandler;

protected BaseHolder(Source source)
{
Expand Down Expand Up @@ -101,7 +101,7 @@ public void doStart()
public void doStop()
throws Exception
{
if (!_extInstance)
if (_instance == null)
_class = null;
}

Expand Down Expand Up @@ -163,12 +163,26 @@ protected void illegalStateIfContextStarted()
}
}

protected synchronized void setInstance(T instance)
{
_instance = instance;
if (instance == null)
setHeldClass(null);
else
setHeldClass((Class<T>)instance.getClass());
}

protected synchronized T getInstance()
{
return _instance;
}

/**
* @return True if this holder was created for a specific instance.
*/
public boolean isInstance()
public synchronized boolean isInstance()
{
return _extInstance;
return _instance != null;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,27 +91,29 @@ public void doStart()
{
super.doStart();

if (!javax.servlet.Filter.class
.isAssignableFrom(_class))
if (!javax.servlet.Filter.class.isAssignableFrom(getHeldClass()))
{
String msg = _class + " is not a javax.servlet.Filter";
super.stop();
String msg = getHeldClass() + " is not a javax.servlet.Filter";
doStop();
joakime marked this conversation as resolved.
Show resolved Hide resolved
throw new IllegalStateException(msg);
}
}

@Override
public void initialize() throws Exception
{
if (!_initialized)
synchronized (this)
{
super.initialize();
if (_filter != null)
return;

super.initialize();
_filter = getInstance();
if (_filter == null)
{
try
{
ServletContext context = _servletHandler.getServletContext();
ServletContext context = getServletHandler().getServletContext();
_filter = (context instanceof ServletContextHandler.Context)
? context.createFilter(getHeldClass())
: getHeldClass().getDeclaredConstructor().newInstance();
Expand All @@ -126,37 +128,30 @@ public void initialize() throws Exception
throw ex;
}
}

_config = new Config();
if (LOG.isDebugEnabled())
LOG.debug("Filter.init {}", _filter);
_filter.init(_config);
}

_initialized = true;
}

@Override
public void doStop()
throws Exception
{
super.doStop();
_config = null;
if (_filter != null)
{
try
{
destroyInstance(_filter);
}
catch (Exception e)
finally
{
LOG.warn(e);
_filter = null;
}
}
if (!_extInstance)
_filter = null;

_config = null;
_initialized = false;
super.doStop();
}

@Override
Expand All @@ -172,11 +167,7 @@ public void destroyInstance(Object o)

public synchronized void setFilter(Filter filter)
{
_filter = filter;
_extInstance = true;
setHeldClass(filter.getClass());
if (getName() == null)
setName(filter.getClass().getName());
setInstance(filter);
}

public Filter getFilter()
Expand All @@ -187,19 +178,19 @@ public Filter getFilter()
@Override
public void dump(Appendable out, String indent) throws IOException
{
if (_initParams.isEmpty())
if (getInitParameters().isEmpty())
Dumpable.dumpObjects(out, indent, this,
_filter == null ? getHeldClass() : _filter);
else
Dumpable.dumpObjects(out, indent, this,
_filter == null ? getHeldClass() : _filter,
new DumpableCollection("initParams", _initParams.entrySet()));
new DumpableCollection("initParams", getInitParameters().entrySet()));
}

@Override
public String toString()
{
return String.format("%s@%x==%s,inst=%b,async=%b", _name, hashCode(), _className, _filter != null, isAsyncSupported());
return String.format("%s@%x==%s,inst=%b,async=%b", getName(), hashCode(), getClassName(), _filter != null, isAsyncSupported());
}

public FilterRegistration.Dynamic getRegistration()
Expand All @@ -220,9 +211,9 @@ public void addMappingForServletNames(EnumSet<DispatcherType> dispatcherTypes, b
mapping.setServletNames(servletNames);
mapping.setDispatcherTypes(dispatcherTypes);
if (isMatchAfter)
_servletHandler.addFilterMapping(mapping);
getServletHandler().addFilterMapping(mapping);
else
_servletHandler.prependFilterMapping(mapping);
getServletHandler().prependFilterMapping(mapping);
}

@Override
Expand All @@ -234,15 +225,15 @@ public void addMappingForUrlPatterns(EnumSet<DispatcherType> dispatcherTypes, bo
mapping.setPathSpecs(urlPatterns);
mapping.setDispatcherTypes(dispatcherTypes);
if (isMatchAfter)
_servletHandler.addFilterMapping(mapping);
getServletHandler().addFilterMapping(mapping);
else
_servletHandler.prependFilterMapping(mapping);
getServletHandler().prependFilterMapping(mapping);
}

@Override
public Collection<String> getServletNameMappings()
{
FilterMapping[] mappings = _servletHandler.getFilterMappings();
FilterMapping[] mappings = getServletHandler().getFilterMappings();
List<String> names = new ArrayList<String>();
for (FilterMapping mapping : mappings)
{
Expand All @@ -258,7 +249,7 @@ public Collection<String> getServletNameMappings()
@Override
public Collection<String> getUrlPatternMappings()
{
FilterMapping[] mappings = _servletHandler.getFilterMappings();
FilterMapping[] mappings = getServletHandler().getFilterMappings();
List<String> patterns = new ArrayList<String>();
for (FilterMapping mapping : mappings)
{
Expand All @@ -277,7 +268,7 @@ class Config extends HolderConfig implements FilterConfig
@Override
public String getFilterName()
{
return _name;
return getName();
}
}
}
23 changes: 15 additions & 8 deletions jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Holder.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,15 @@ public abstract class Holder<T> extends BaseHolder<T>
{
private static final Logger LOG = Log.getLogger(Holder.class);

protected final Map<String, String> _initParams = new HashMap<String, String>(3);
protected String _displayName;
protected boolean _asyncSupported;
protected String _name;
protected boolean _initialized = false;
private final Map<String, String> _initParams = new HashMap<String, String>(3);
private String _displayName;
private boolean _asyncSupported;
private String _name;

protected Holder(Source source)
{
super(source);
switch (_source.getOrigin())
switch (getSource().getOrigin())
{
case JAVAX_API:
case DESCRIPTOR:
Expand Down Expand Up @@ -98,6 +97,14 @@ public String getName()
return _name;
}

@Override
protected synchronized void setInstance(T instance)
{
super.setInstance(instance);
if (getName() == null)
setName(String.format("%s@%x", instance.getClass().getName(), instance.hashCode()));
}

public void destroyInstance(Object instance)
throws Exception
{
Expand Down Expand Up @@ -175,15 +182,15 @@ public String dump()
@Override
public String toString()
{
return String.format("%s@%x==%s", _name, hashCode(), _className);
return String.format("%s@%x==%s", _name, hashCode(), getClassName());
}

protected class HolderConfig
{

public ServletContext getServletContext()
{
return _servletHandler.getServletContext();
return getServletHandler().getServletContext();
}

public String getInitParameter(String param)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,52 +64,66 @@ public EventListener getListener()
*/
public void setListener(EventListener listener)
{
_listener = listener;
_extInstance = true;
setHeldClass(_listener.getClass());
setInstance(listener);
}

@Override
public void doStart() throws Exception
{
super.doStart();
if (!java.util.EventListener.class.isAssignableFrom(_class))
if (!java.util.EventListener.class.isAssignableFrom(getHeldClass()))
{
String msg = _class + " is not a java.util.EventListener";
String msg = getHeldClass() + " is not a java.util.EventListener";
super.stop();
throw new IllegalStateException(msg);
}

ContextHandler contextHandler = ContextHandler.getCurrentContext().getContextHandler();
if (_listener == null)
if (contextHandler != null)
{
//create an instance of the listener and decorate it
try
{
ServletContext scontext = contextHandler.getServletContext();
_listener = (scontext instanceof ServletContextHandler.Context)
? scontext.createListener(getHeldClass())
: getHeldClass().getDeclaredConstructor().newInstance();
}
catch (ServletException ex)
_listener = getInstance();
if (_listener == null)
{
Throwable cause = ex.getRootCause();
if (cause instanceof InstantiationException)
throw (InstantiationException)cause;
if (cause instanceof IllegalAccessException)
throw (IllegalAccessException)cause;
throw ex;
//create an instance of the listener and decorate it
try
{
ServletContext scontext = contextHandler.getServletContext();
_listener = (scontext instanceof ServletContextHandler.Context)
? scontext.createListener(getHeldClass())
: getHeldClass().getDeclaredConstructor().newInstance();
}
catch (ServletException ex)
{
Throwable cause = ex.getRootCause();
if (cause instanceof InstantiationException)
throw (InstantiationException)cause;
if (cause instanceof IllegalAccessException)
throw (IllegalAccessException)cause;
throw ex;
}
}
contextHandler.addEventListener(_listener);
}
contextHandler.addEventListener(_listener);
}

@Override
public void doStop() throws Exception
{
super.doStop();
if (!_extInstance)
_listener = null;
if (_listener != null)
{
try
{
ContextHandler contextHandler = ContextHandler.getCurrentContext().getContextHandler();
if (contextHandler != null)
contextHandler.removeEventListener(_listener);
getServletHandler().destroyListener(_listener);
}
finally
{
_listener = null;
}
}
}

@Override
Expand Down
Loading