You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Jeremy Boynes <jb...@apache.org> on 2013/09/19 17:31:35 UTC

SCI discovery ordering

On Sep 19, 2013, at 6:07 AM, markt@apache.org wrote:

> Author: markt
> Date: Thu Sep 19 13:07:02 2013
> New Revision: 1524727
> 
> URL: http://svn.apache.org/r1524727
> Log:
> Always load container SCIs first

This seems at odds with the requirement that "the order in which these services are discovered MUST follow the application’s class loading delegation model" which is independent of orderedLibs. If we base this on ServiceLoader, which seems to be the intent, the algorithm would be:

if (orderedLibs != null)
  excludedJars = URLs of all jars in WEB-INF/lib that are not listed in orderedLibs
for (URL configFile : webappLoader.getResources()) {
 if (!excludedJars.contains(jar containing configFile)) {
   load SCIs from entries in configFile
 }
}

I originally shied away from trying to compare the URLs returned by the classloader and with those from ServletContext.getResource() as I was was not sure they would be the same. If they are, then the implementation can be simplified along the lines above. If not, we would also need to pass in the delegation order of the classloader to know whether to add the "application" SCIs before or after the "container" ones.

I'll go ahead and modify this based on the assumption the URLs can be compared.

Cheers
Jeremy


Re: SCI discovery ordering

Posted by Remy Maucherat <re...@apache.org>.
On Fri, 2013-09-20 at 08:38 +0100, Mark Thomas wrote:
> a) The container defined SCIs need to be loaded first. Any other
> solution places an unnecessary burden on application developers. It is
> reasonable for application developers to expect that if a container
> advertises support for WebSocket, JSP, etc. then those features are
> available when the application's SCI's are executed.

I agree the SCIs that may be used for core functionality should be
loaded first. Since that's now the design choice for JSP and WS, that
would make 1) JSP 2) WS 3) anything else.

Rémy



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


SERVLET_SPEC-79, was: SCI discovery ordering

Posted by Jeremy Boynes <jb...@apache.org>.
On Sep 20, 2013, at 2:43 PM, Nick Williams <ni...@nicholaswilliams.net> wrote:

> 
> On Sep 20, 2013, at 4:33 PM, Jeremy Boynes wrote:
> 
>> On Fri, Sep 20, 2013 at 8:50 AM, Mark Thomas <ma...@apache.org> wrote:
>> 
>>> On 20/09/2013 16:02, Jeremy Boynes wrote:
>>>> The only ordering concern for SCIs in the spec is that they are
>>>> "discovered" following the classloader delegation model. This will
>>>> typically be configured to load application classes first,
>>>> something r1524727 does not do.
>>> 
>>> The intention of the language around discovery is to clarify the
>>> expected behaviour when both the container and the application provide
>>> an implementation of the same SCI. As with any other class, the
>>> delegation model adopted by the application must be used.
>>> 
>>> It has no bearing on the order in which one SCI implementation is
>>> loaded with respect to another SCI.
>>> 
>>> It has no bearing on the order in which one SCI implementation is
>>> invoked with respect to another SCI.
>>> 
>>> r1524727 is fully compliant with the Servlet spec.
>> 
>> 
>> Thanks for clarifying. It still leaves the issue related to ordering and
>> also to duplicate functionality.
>> 
>> For example, if both the application and the container use an SCI to
>> bootstrap JAX-WS functionality, which should be used? The deployer can
>> exclude the implementation in the application (using absolute ordering) but
>> she cannot make that usable in preference to the one in the container
>> unless they happen use the same implementation and SCI class. There is no
>> mechanism to exclude the implementation provided by the container and use
>> an equivalent but different mechanism included in the application.
>> 
>> The SCIs we use (JSP and WS) are independent and can be called in any
>> order. The same is true of Spring's SCI itself. What triggered this issue
>> is application code being called by Spring's SCI that adds an implicit
>> coupling to WS, whereas WS says the application should be using a listener.
>> 
>> To date, there is no presumption of ordering of SCI invocation in the spec
>> and so frameworks need to code defensively. I agree this is a burden on the
>> framework and limits the deployer's flexibility. To address that I'm saying
>> we need a more sophisticated mechanism for ordering/excluding SCIs akin to
>> (but separate from) that used for ordering fragments. Do you see any issue
>> with such a mechanism?
> 
> https://java.net/jira/browse/SERVLET_SPEC-79
> 
> My proposal in that issue, which I stand by, is to use web-fragment ordering for SCI ordering. Importantly, this supports ordering without changing the API or XML schema, so Servlet 3.0 and 3.1 can be *encouraged* to implement SCI ordering retroactively. I've already convinced the Jetty team to take the same approach Tomcat is taking. JBoss, WebLogic, and WebSphere already use web-fragment ordering.

I think that's a good option when all SCIs are in named web fragments. However, there are two cases it does not cover:
1) when the SCI is declared in an anonymous web fragment (a jar in WEB-INF/lib without an XML DD)
2) when the SCI is declared by the container

AIUI, the other containers name anonymous fragments using the basename of the jar, allowing them to be referenced in an ordering clause more specifically than as "others." Tomcat, however, uses the actual URL of the jar which is not known to the deployer. We would need to change our naming mechanism to allow that, and the spec would want to codify a way to reference anonymous fragments.

This still would not allow ordering to reference SCIs declared by the container as a portable application cannot reference such a container specific artifacts and so cannot include them in ordering.

Cheers
Jeremy


Re: SCI discovery ordering

Posted by Nick Williams <ni...@nicholaswilliams.net>.
On Sep 20, 2013, at 4:33 PM, Jeremy Boynes wrote:

> On Fri, Sep 20, 2013 at 8:50 AM, Mark Thomas <ma...@apache.org> wrote:
> 
>> On 20/09/2013 16:02, Jeremy Boynes wrote:
>>> The only ordering concern for SCIs in the spec is that they are
>>> "discovered" following the classloader delegation model. This will
>>> typically be configured to load application classes first,
>>> something r1524727 does not do.
>> 
>> The intention of the language around discovery is to clarify the
>> expected behaviour when both the container and the application provide
>> an implementation of the same SCI. As with any other class, the
>> delegation model adopted by the application must be used.
>> 
>> It has no bearing on the order in which one SCI implementation is
>> loaded with respect to another SCI.
>> 
>> It has no bearing on the order in which one SCI implementation is
>> invoked with respect to another SCI.
>> 
>> r1524727 is fully compliant with the Servlet spec.
> 
> 
> Thanks for clarifying. It still leaves the issue related to ordering and
> also to duplicate functionality.
> 
> For example, if both the application and the container use an SCI to
> bootstrap JAX-WS functionality, which should be used? The deployer can
> exclude the implementation in the application (using absolute ordering) but
> she cannot make that usable in preference to the one in the container
> unless they happen use the same implementation and SCI class. There is no
> mechanism to exclude the implementation provided by the container and use
> an equivalent but different mechanism included in the application.
> 
> The SCIs we use (JSP and WS) are independent and can be called in any
> order. The same is true of Spring's SCI itself. What triggered this issue
> is application code being called by Spring's SCI that adds an implicit
> coupling to WS, whereas WS says the application should be using a listener.
> 
> To date, there is no presumption of ordering of SCI invocation in the spec
> and so frameworks need to code defensively. I agree this is a burden on the
> framework and limits the deployer's flexibility. To address that I'm saying
> we need a more sophisticated mechanism for ordering/excluding SCIs akin to
> (but separate from) that used for ordering fragments. Do you see any issue
> with such a mechanism?

https://java.net/jira/browse/SERVLET_SPEC-79

My proposal in that issue, which I stand by, is to use web-fragment ordering for SCI ordering. Importantly, this supports ordering without changing the API or XML schema, so Servlet 3.0 and 3.1 can be *encouraged* to implement SCI ordering retroactively. I've already convinced the Jetty team to take the same approach Tomcat is taking. JBoss, WebLogic, and WebSphere already use web-fragment ordering.

N


Re: SCI discovery ordering

Posted by Jeremy Boynes <jb...@apache.org>.
On Fri, Sep 20, 2013 at 8:50 AM, Mark Thomas <ma...@apache.org> wrote:

> On 20/09/2013 16:02, Jeremy Boynes wrote:
> > The only ordering concern for SCIs in the spec is that they are
> > "discovered" following the classloader delegation model. This will
> > typically be configured to load application classes first,
> > something r1524727 does not do.
>
> The intention of the language around discovery is to clarify the
> expected behaviour when both the container and the application provide
> an implementation of the same SCI. As with any other class, the
> delegation model adopted by the application must be used.
>
> It has no bearing on the order in which one SCI implementation is
> loaded with respect to another SCI.
>
> It has no bearing on the order in which one SCI implementation is
> invoked with respect to another SCI.
>
> r1524727 is fully compliant with the Servlet spec.


Thanks for clarifying. It still leaves the issue related to ordering and
also to duplicate functionality.

For example, if both the application and the container use an SCI to
bootstrap JAX-WS functionality, which should be used? The deployer can
exclude the implementation in the application (using absolute ordering) but
she cannot make that usable in preference to the one in the container
unless they happen use the same implementation and SCI class. There is no
mechanism to exclude the implementation provided by the container and use
an equivalent but different mechanism included in the application.

The SCIs we use (JSP and WS) are independent and can be called in any
order. The same is true of Spring's SCI itself. What triggered this issue
is application code being called by Spring's SCI that adds an implicit
coupling to WS, whereas WS says the application should be using a listener.

To date, there is no presumption of ordering of SCI invocation in the spec
and so frameworks need to code defensively. I agree this is a burden on the
framework and limits the deployer's flexibility. To address that I'm saying
we need a more sophisticated mechanism for ordering/excluding SCIs akin to
(but separate from) that used for ordering fragments. Do you see any issue
with such a mechanism?

Re: SCI discovery ordering

Posted by Mark Thomas <ma...@apache.org>.
On 22/09/2013 10:55, Jeremy Boynes wrote:
> On Sep 22, 2013, at 1:44 AM, Mark Thomas <ma...@apache.org> wrote:
> 
>> On 22/09/2013 00:27, Jeremy Boynes wrote:
>> 
>>> As a concrete example of how this impacts the behaviour, consider
>>> the case where the application includes its own JSP engine. With
>>> the RI's delegation model, the application's engine's SCI would
>>> execute first allowing it to register a Servlet and mapping to
>>> handle the "*.jsp" pattern. When the container's engine's SCI was
>>> executed, that mapping would already be bound and could not be
>>> pre-empted (engines already need to allow for that in case the
>>> application configured that mapping in its web.xml). If the
>>> container is always given priority, then the container's engine
>>> would be used rather than the one the application intended.
>> 
>> No. It would still be loaded from the application (for that
>> webapp).
> 
> For two versions of the *same* implementation, yes.

Agreed. This is the case the EG was trying to cover. Unfortunately that
is just a small part of the total grey area that could do with some
clarification.

> But if they used
> *different* implementations of the *same functionality,* the
> container's would always get precedence. For example, if an
> application included Tyrus's WebSocket implementation it would always
> be invoked after Tomcat's. Or for JAX-RS, if the container was
> configured with CXF and the application included Jersey, Jersey would
> not be able to register its Servlets for the REST endpoints as CXF
> would have already mapped them.

Some time ago, Tomcat allowed users to include container JARs in
absolute orderings to handle this sort of situation. However, the EG was
very clear that that behaviour was not permitted and that absolute
ordering could only include applications JARs.

I'm wondering if reverting to something like this might be the way to
solve this. We shall see...

> The issue here is that programmatic registrations cannot modify the
> existing configuration. Once a framework has registered a servlet,
> filter, listener or mapping it cannot be replaced by another.
> Frameworks that applications bundle in WEB-INF/lib need to have a
> chance to perform their initialization before an equivalent but
> different implementation provided by the container.

This is starting to get really messy. There are enough different edge
cases that I'm not such I am keeping track of them all. I'm also not
sure that SERVLET_SPEC-79 has captured them all either.

Can I suggest the following approach:
- start a new wiki page listing all the edge cases / problems / issues
we can think of
- discuss some solutions to those issues on list  - more complex
proposals can be documented on the wiki if necessary
- document the proposal(s) on the wiki
- update SERVLET_SPEC-79 with the full set of issues and solution(s)

At that point we can figure out what minimal changes we can make to
Tomcat 8 to align it as far as possible to the proposed solution. What I
want to avoid is a large / invasive change to Tomcat to align it to the
proposed solution only to have to undo / redo it if the EG opts for a
different (or worse still no) solution.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: SCI discovery ordering

Posted by Jeremy Boynes <jb...@apache.org>.
On Sep 22, 2013, at 10:55 AM, Jeremy Boynes <jb...@apache.org> wrote:

> On Sep 22, 2013, at 1:44 AM, Mark Thomas <ma...@apache.org> wrote:
> 
>> On 22/09/2013 00:27, Jeremy Boynes wrote:
>> 
>>> As a concrete example of how this impacts the behaviour, consider the case where the application includes its own JSP engine. With the RI's delegation model, the application's engine's SCI would execute first allowing it to register a Servlet and mapping to handle the "*.jsp" pattern. When the container's engine's SCI was executed, that mapping would already be bound and could not be pre-empted (engines already need to allow for that in case the application configured that mapping in its web.xml). If the container is always given priority, then the container's engine would be used rather than the one the application intended.
>> 
>> No. It would still be loaded from the application (for that webapp).
> 
> For two versions of the *same* implementation, yes. But if they used *different* implementations of the *same functionality,* the container's would always get precedence. For example, if an application included Tyrus's WebSocket implementation it would always be invoked after Tomcat's. Or for JAX-RS, if the container was configured with CXF and the application included Jersey, Jersey would not be able to register its Servlets for the REST endpoints as CXF would have already mapped them.
> 
> The issue here is that programmatic registrations cannot modify the existing configuration. Once a framework has registered a servlet, filter, listener or mapping it cannot be replaced by another. Frameworks that applications bundle in WEB-INF/lib need to have a chance to perform their initialization before an equivalent but different implementation provided by the container.

Patch to avoid this by following classloader delegation order … 

Re: SCI discovery ordering

Posted by Jeremy Boynes <jb...@apache.org>.
On Sep 22, 2013, at 1:44 AM, Mark Thomas <ma...@apache.org> wrote:

> On 22/09/2013 00:27, Jeremy Boynes wrote:
> 
>> As a concrete example of how this impacts the behaviour, consider the case where the application includes its own JSP engine. With the RI's delegation model, the application's engine's SCI would execute first allowing it to register a Servlet and mapping to handle the "*.jsp" pattern. When the container's engine's SCI was executed, that mapping would already be bound and could not be pre-empted (engines already need to allow for that in case the application configured that mapping in its web.xml). If the container is always given priority, then the container's engine would be used rather than the one the application intended.
> 
> No. It would still be loaded from the application (for that webapp).

For two versions of the *same* implementation, yes. But if they used *different* implementations of the *same functionality,* the container's would always get precedence. For example, if an application included Tyrus's WebSocket implementation it would always be invoked after Tomcat's. Or for JAX-RS, if the container was configured with CXF and the application included Jersey, Jersey would not be able to register its Servlets for the REST endpoints as CXF would have already mapped them.

The issue here is that programmatic registrations cannot modify the existing configuration. Once a framework has registered a servlet, filter, listener or mapping it cannot be replaced by another. Frameworks that applications bundle in WEB-INF/lib need to have a chance to perform their initialization before an equivalent but different implementation provided by the container.

--
Jeremy


Re: SCI discovery ordering

Posted by Mark Thomas <ma...@apache.org>.
On 22/09/2013 00:27, Jeremy Boynes wrote:
> If the intent of the language in the spec is to give the application implementation priority if it comes first in the delegation order, this should apply to "discovery" as well as classloading per the ServiceLoader semantic.

No, the intent of the language was only to make clear that the
delegation order must be honoured when loading SCI implementations.

To quote from the Servlet EG discussion on this topic:

<quote>
If the same ServletContainerInitializer is both on the container classpath
and in a jar in WEB-INF/lib, which one should be used?

Will this be by the normal inverted classloading priority
of web applications?
</quote>

The result of the discussion was "Yes." and the language was added to
the end of 8.2. It went on to discuss some edge cases but no conclusion
was reached and no further clarification was added to the spec.


> r1524727's scheme is more nuanced:
> * if orderedLibs is present, it loads and executes in the following order:
>   1) SCIs defined in the webapp's parent classloader ("container" SCIs) in undefined order
>   2) SCIs defined by jars in orderedLibs ("application" SCIs) in orderedLibs order
> * else, if the webapp loader is application first:
>   1) SCIs defined by the application classloader in undefined order
>   2) SCIs defined by the webapp's parent classloader in undefined order
> * else, as the webapp loader is parent first:
>   1) SCIs defined by the webapp's parent classloader in undefined order
>   2) SCIs defined by the application classloader in undefined order

I'll take another look at r1524727 as that isn't want was intended. What
should happen is:
- List all SCIs defined by the container
- List all SCIs defined by the application (may only be from a subset of
JARs if absolute ordering is present)
- Load all the SCIs defined by the container (note some may be loaded
from the application if the application ships with an alternative
implementation and the application first delegation model is being used)
- Load all the SCIs defined by the application


> As a concrete example of how this impacts the behaviour, consider the case where the application includes its own JSP engine. With the RI's delegation model, the application's engine's SCI would execute first allowing it to register a Servlet and mapping to handle the "*.jsp" pattern. When the container's engine's SCI was executed, that mapping would already be bound and could not be pre-empted (engines already need to allow for that in case the application configured that mapping in its web.xml). If the container is always given priority, then the container's engine would be used rather than the one the application intended.

No. It would still be loaded from the application (for that webapp).

> The spec is very clear that SCIs are intended for frameworks and not for general application initialization (which is covered by context listeners and fragment ordering).

I disagree. The specification uses adding frameworks as an example of a
use case the an SCI can support, but nowhere does it say that it may not
be used by applications or that it isn't intended for applications.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: SCI discovery ordering

Posted by Jeremy Boynes <jb...@apache.org>.
On Sep 20, 2013, at 8:50 AM, Mark Thomas <ma...@apache.org> wrote:

> On 20/09/2013 16:02, Jeremy Boynes wrote:
>> The only ordering concern for SCIs in the spec is that they are 
>> "discovered" following the classloader delegation model. This will 
>> typically be configured to load application classes first,
>> something r1524727 does not do.
> 
> The intention of the language around discovery is to clarify the
> expected behaviour when both the container and the application provide
> an implementation of the same SCI. As with any other class, the
> delegation model adopted by the application must be used.
> 
> It has no bearing on the order in which one SCI implementation is
> loaded with respect to another SCI.

Thinking more here, the spec refers to the "service lookup mechanism" which we've taken as being java.util.ServiceLoader. The semantics of that, returning services using an Iterator<S>, means services must be loaded and instantiated to return them. ServiceLoader consistently returns services in instantiation order. That has bearing as it determines the order in which class initializers and constructors are called.

> It has no bearing on the order in which one SCI implementation is
> invoked with respect to another SCI.

Given "discovery" and loading is ordered, I think it's reasonable for instances to be invoked in the same order.

> r1524727 is fully compliant with the Servlet spec.

Perhaps with the letter but it seems at odds with the intent. If frameworks with SCIs are to be treated in the same way as other extensions they should follow the classloader delegation model with container supplied versions prioritized before *or after* application supplied versions per that model.

If the intent of the language in the spec is to give the application implementation priority if it comes first in the delegation order, this should apply to "discovery" as well as classloading per the ServiceLoader semantic. As noted in JIRA[1] for the reference implementation, GlassFish loads *and executes* SCIs in the order they are "discovered" by ServiceLoader, following the priority of the delegation model.

r1524727's scheme is more nuanced:
* if orderedLibs is present, it loads and executes in the following order:
  1) SCIs defined in the webapp's parent classloader ("container" SCIs) in undefined order
  2) SCIs defined by jars in orderedLibs ("application" SCIs) in orderedLibs order
* else, if the webapp loader is application first:
  1) SCIs defined by the application classloader in undefined order
  2) SCIs defined by the webapp's parent classloader in undefined order
* else, as the webapp loader is parent first:
  1) SCIs defined by the webapp's parent classloader in undefined order
  2) SCIs defined by the application classloader in undefined order

As a concrete example of how this impacts the behaviour, consider the case where the application includes its own JSP engine. With the RI's delegation model, the application's engine's SCI would execute first allowing it to register a Servlet and mapping to handle the "*.jsp" pattern. When the container's engine's SCI was executed, that mapping would already be bound and could not be pre-empted (engines already need to allow for that in case the application configured that mapping in its web.xml). If the container is always given priority, then the container's engine would be used rather than the one the application intended.

This example can be applied equally to any WebSocket, JAX-WS or JAX-RS framework that is provided by both an application and the container. That's especially important for the hosted case where applications may require a specific implementation of a framework be used and where they would not be able to modify those provided by the container.

The spec is very clear that SCIs are intended for frameworks and not for general application initialization (which is covered by context listeners and fragment ordering).

This basically comes down the the following sequence for initialization:
1) load and merge web.xml and all web-fragment.xml, applying fragment ordering
2) load and invoke framework SCIs in delegation order so they can add to the effective web.xml
3) add configuration from container's default web.xml
4) initialize application by calling SCLs in web.xml order (they may in turn add servlets and filters)
5) initialize servlets and filters in load-on-startup order

Cheers
Jeremy

[1] https://java.net/jira/browse/GLASSFISH-20788


Re: SCI discovery ordering

Posted by Mark Thomas <ma...@apache.org>.
On 20/09/2013 16:02, Jeremy Boynes wrote:
> The only ordering concern for SCIs in the spec is that they are 
> "discovered" following the classloader delegation model. This will 
> typically be configured to load application classes first,
> something r1524727 does not do.

The intention of the language around discovery is to clarify the
expected behaviour when both the container and the application provide
an implementation of the same SCI. As with any other class, the
delegation model adopted by the application must be used.

It has no bearing on the order in which one SCI implementation is
loaded with respect to another SCI.

It has no bearing on the order in which one SCI implementation is
invoked with respect to another SCI.

r1524727 is fully compliant with the Servlet spec.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: SCI discovery ordering

Posted by Jeremy Boynes <jb...@apache.org>.
On Sep 20, 2013, at 12:38 AM, Mark Thomas <ma...@apache.org> wrote:

> On 20/09/2013 06:11, Jeremy Boynes wrote:
>> On Sep 19, 2013, at 8:31 AM, Jeremy Boynes <jb...@apache.org> wrote:
>> 
>>> On Sep 19, 2013, at 6:07 AM, markt@apache.org wrote:
>>> 
>>>> Author: markt
>>>> Date: Thu Sep 19 13:07:02 2013
>>>> New Revision: 1524727
>>>> 
>>>> URL: http://svn.apache.org/r1524727
>>>> Log:
>>>> Always load container SCIs first
>>> 
>>> This seems at odds with the requirement that "the order in which these services are discovered MUST follow the application’s class loading delegation model" which is independent of orderedLibs. If we base this on ServiceLoader, which seems to be the intent, the algorithm would be:
>>> 
>>> if (orderedLibs != null)
>>> excludedJars = URLs of all jars in WEB-INF/lib that are not listed in orderedLibs
>>> for (URL configFile : webappLoader.getResources()) {
>>> if (!excludedJars.contains(jar containing configFile)) {
>>>  load SCIs from entries in configFile
>>> }
>>> }
>>> 
>>> I originally shied away from trying to compare the URLs returned by the classloader and with those from ServletContext.getResource() as I was was not sure they would be the same. If they are, then the implementation can be simplified along the lines above. If not, we would also need to pass in the delegation order of the classloader to know whether to add the "application" SCIs before or after the "container" ones.
>>> 
>>> I'll go ahead and modify this based on the assumption the URLs can be compared.
>> 
>> Patch that changes this to load in the order returned by the ClassLoader
> 
> -1
> 
> a) The container defined SCIs need to be loaded first. Any other
> solution places an unnecessary burden on application developers. It is
> reasonable for application developers to expect that if a container
> advertises support for WebSocket, JSP, etc. then those features are
> available when the application's SCI's are executed.

I respectfully disagree with this. The spec consistently refers to SCIs as a mechanism for extending the container and not as an application API. The examples it contains refer to frameworks such as "JAX-WS, JAX-RS and JSF" not application code; and there is, of course, the whole section on JSP pluggability. The HandlesTypes mechanism allows frameworks to configure servlet components (servlets, filters, listeners) based on annotations on application code; it is not intended for application code itself. The registration API allows components to be "preliminary" when SCIs are executing and does not guarantee they are available. And so on...

IOW, the ambiguity in SCI ordering is an issue for framework developers not application developers.

Further, in the original user's case they were trying to access a WebSocket ServerContainer. To quote the WebSocket specification:
> "When running on the web container, the addEndpoint methods may be called from a javax.servlet.ServletContextListener provided by the developer and configured in the deployment descriptor of the web application."
it specifically refers to use in a listener rather than an SCI. IMO, this is an application bug, one that is easily corrected by following the WebSocket spec and which does not need container changes.


> b) The URls are not (yet) comparable. Further refactoring of the new
> resources implementation is required before this is true in all cases.
> 
> c) The Servlet specification currently makes no statement on a required
> execution order - therefore any ordering scheme is specification
> compliant. From a specification point of view, both the current code and
> the proposed patch are acceptable.

The only ordering concern for SCIs in the spec is that they are "discovered" following the classloader delegation model. This will typically be configured to load application classes first, something r1524727 does not do. We may choose to invoke SCIs in a different order, but we need to "discover" them correctly.

If we are going to provide an ordering mechanism for SCIs, then it needs to be more robust to support cases such as Remy's where there are ordering rules at a finer granularity, such as JSP before WebSocket or vice versa. Something similar but separate to fragment ordering that applies specifically to SCIs.

> d) No-one on the EG has yet raised any objections to the proposal to
> require container defined SCIs to be loaded first.

I am not privy to EG discussions so cannot comment. If I was participating, I would raise the same objections there.

> I have plans to address b) as it should allow the simplification of the
> resources handling in the WebappClassLoader but even with that issue
> addressed the other points above stand.

As I've said, I don't like any mechanism relying on matching URLs either. The exclusion cause for SCI discovery is ambiguous and a weird coupling of the fragment ordering mechanism with SCI service loading and if anything I would hope the EG re-evaluates that in 3.2 as well, especially as they expect that to be based on the semantics of the ServiceLoader mechanism.

Cheers
Jeremy


Re: SCI discovery ordering

Posted by Mark Thomas <ma...@apache.org>.
On 20/09/2013 06:11, Jeremy Boynes wrote:
> On Sep 19, 2013, at 8:31 AM, Jeremy Boynes <jb...@apache.org> wrote:
> 
>> On Sep 19, 2013, at 6:07 AM, markt@apache.org wrote:
>>
>>> Author: markt
>>> Date: Thu Sep 19 13:07:02 2013
>>> New Revision: 1524727
>>>
>>> URL: http://svn.apache.org/r1524727
>>> Log:
>>> Always load container SCIs first
>>
>> This seems at odds with the requirement that "the order in which these services are discovered MUST follow the application’s class loading delegation model" which is independent of orderedLibs. If we base this on ServiceLoader, which seems to be the intent, the algorithm would be:
>>
>> if (orderedLibs != null)
>>  excludedJars = URLs of all jars in WEB-INF/lib that are not listed in orderedLibs
>> for (URL configFile : webappLoader.getResources()) {
>> if (!excludedJars.contains(jar containing configFile)) {
>>   load SCIs from entries in configFile
>> }
>> }
>>
>> I originally shied away from trying to compare the URLs returned by the classloader and with those from ServletContext.getResource() as I was was not sure they would be the same. If they are, then the implementation can be simplified along the lines above. If not, we would also need to pass in the delegation order of the classloader to know whether to add the "application" SCIs before or after the "container" ones.
>>
>> I'll go ahead and modify this based on the assumption the URLs can be compared.
> 
> Patch that changes this to load in the order returned by the ClassLoader

-1

a) The container defined SCIs need to be loaded first. Any other
solution places an unnecessary burden on application developers. It is
reasonable for application developers to expect that if a container
advertises support for WebSocket, JSP, etc. then those features are
available when the application's SCI's are executed.

b) The URls are not (yet) comparable. Further refactoring of the new
resources implementation is required before this is true in all cases.

c) The Servlet specification currently makes no statement on a required
execution order - therefore any ordering scheme is specification
compliant. From a specification point of view, both the current code and
the proposed patch are acceptable.

d) No-one on the EG has yet raised any objections to the proposal to
require container defined SCIs to be loaded first.

I have plans to address b) as it should allow the simplification of the
resources handling in the WebappClassLoader but even with that issue
addressed the other points above stand.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: SCI discovery ordering

Posted by Jeremy Boynes <jb...@apache.org>.
On Sep 19, 2013, at 8:31 AM, Jeremy Boynes <jb...@apache.org> wrote:

> On Sep 19, 2013, at 6:07 AM, markt@apache.org wrote:
> 
>> Author: markt
>> Date: Thu Sep 19 13:07:02 2013
>> New Revision: 1524727
>> 
>> URL: http://svn.apache.org/r1524727
>> Log:
>> Always load container SCIs first
> 
> This seems at odds with the requirement that "the order in which these services are discovered MUST follow the application’s class loading delegation model" which is independent of orderedLibs. If we base this on ServiceLoader, which seems to be the intent, the algorithm would be:
> 
> if (orderedLibs != null)
>  excludedJars = URLs of all jars in WEB-INF/lib that are not listed in orderedLibs
> for (URL configFile : webappLoader.getResources()) {
> if (!excludedJars.contains(jar containing configFile)) {
>   load SCIs from entries in configFile
> }
> }
> 
> I originally shied away from trying to compare the URLs returned by the classloader and with those from ServletContext.getResource() as I was was not sure they would be the same. If they are, then the implementation can be simplified along the lines above. If not, we would also need to pass in the delegation order of the classloader to know whether to add the "application" SCIs before or after the "container" ones.
> 
> I'll go ahead and modify this based on the assumption the URLs can be compared.

Patch that changes this to load in the order returned by the ClassLoader


Re: SCI discovery ordering

Posted by Jeremy Boynes <je...@boynes.com>.
On Thu, Sep 19, 2013 at 11:28 AM, Mark Thomas <ma...@apache.org> wrote:

> On 19/09/2013 17:39, Nick Williams wrote:
> >
> > On Sep 19, 2013, at 11:32 AM, Jeremy Boynes wrote:
> >
>
> >> There's a clear model already without assumptions about SCI
> >> ordering: SCIs fire before SCLs, and SCLs can be ordered explicitly
> >> by web.xml. There are no ordering requirements for SCIs except that
> >> they are discovered per the delegation model. This change is
> >> changing that to load "container" SCIs firsts *even if the
> >> delegation model specifies application first.*
> >
> > Loading/discovering SCIs and invoking SCIs are not the same thing.
> > This change is spec compliant because it still loads/discovers SCIs
> > per the delegation model. The spec does NOT say that SCIs must be
> > ordered according to the delegation model (or even that they must be
> > ordered in any fashion at all).
>
> Exactly. I went back and checked the EG discussion that lead to the text
> in 8.2. It was solely concerned with loading - not execution order.


My point is is that this change changes that order (which is needed as it
was not actually right to start with). With it, SCIs defined by the
container are always loaded before SCIs defined by the application even if
the application jars precede the container jars in the classloader
delegation order (which they normally would). The algorithm above, however,
ensures that the delegation order is maintained.

As you say, the spec does not specify *any* order for invoking SCIs which
means any conforming SCI implementation cannot assume that it will be
invoked before or after any other. Listener ordering, however, is
determined by the effective web.xml created by merging the fragments and
the ordering rules for that are clearly defined by the spec (including that
they are invoked after all SCIs).

The original issue on the user list was that application code in a SCI was
accessing the ServletContext before it was fully initialized. This is an
application bug which it should fix. One way, as suggested, would be to
defer that work to an SCL that is guaranteed not to be invoked until the
ServletContext is fully initialized; it can do that by registering the SCL
in the SCI or by defining it in web.xml or a fragment if it needs to be
ordered relative to other SCLs.

--
Jeremy

Re: SCI discovery ordering

Posted by Mark Thomas <ma...@apache.org>.
On 19/09/2013 17:39, Nick Williams wrote:
> 
> On Sep 19, 2013, at 11:32 AM, Jeremy Boynes wrote:
> 

>> There's a clear model already without assumptions about SCI
>> ordering: SCIs fire before SCLs, and SCLs can be ordered explicitly
>> by web.xml. There are no ordering requirements for SCIs except that
>> they are discovered per the delegation model. This change is
>> changing that to load "container" SCIs firsts *even if the
>> delegation model specifies application first.*
> 
> Loading/discovering SCIs and invoking SCIs are not the same thing.
> This change is spec compliant because it still loads/discovers SCIs
> per the delegation model. The spec does NOT say that SCIs must be
> ordered according to the delegation model (or even that they must be
> ordered in any fashion at all).

Exactly. I went back and checked the EG discussion that lead to the text
in 8.2. It was solely concerned with loading - not execution order.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: SCI discovery ordering

Posted by Nick Williams <ni...@nicholaswilliams.net>.
On Sep 19, 2013, at 11:32 AM, Jeremy Boynes wrote:

> On Sep 19, 2013, at 8:36 AM, Mark Thomas <ma...@apache.org> wrote:
> 
>> On 19/09/2013 16:31, Jeremy Boynes wrote:
>>> On Sep 19, 2013, at 6:07 AM, markt@apache.org wrote:
>>> 
>>>> Author: markt Date: Thu Sep 19 13:07:02 2013 New Revision:
>>>> 1524727
>>>> 
>>>> URL: http://svn.apache.org/r1524727 Log: Always load container
>>>> SCIs first
>>> 
>>> This seems at odds with the requirement that "the order in which
>>> these services are discovered MUST follow the application’s class
>>> loading delegation model" which is independent of orderedLibs. If
>>> we base this on ServiceLoader, which seems to be the intent, the
>>> algorithm would be:
>> 
>> Nope. See the discussion on the users list.
>> 
>> There is a clear need for container defined SCIs to be executed first
>> and there is nothing (yet) in the Servlet spec that places any
>> constraints on SCI execution order.
> 
> There's a clear model already without assumptions about SCI ordering: SCIs fire before SCLs, and SCLs can be ordered explicitly by web.xml. There are no ordering requirements for SCIs except that they are discovered per the delegation model. This change is changing that to load "container" SCIs firsts *even if the delegation model specifies application first.*

Loading/discovering SCIs and invoking SCIs are not the same thing. This change is spec compliant because it still loads/discovers SCIs per the delegation model. The spec does NOT say that SCIs must be ordered according to the delegation model (or even that they must be ordered in any fashion at all).

> 
> The original issue was that a context attribute was not set when an SCI was run (or actually, as a application class invoked by Spring's SCI was run). As Konstantin pointed out, there is no issue here if that code is run from an SCL rather than the SCI. I argue this is an application issue rather than something the container needs to enforce in a proprietary manner. It is similar to JSP land where Jasper's SCI can only register TLD listener or JSP servlet classes with the context rather than actually instantiating them.
> 
> Further, running container SCIs first may interfere with application SCIs that would be "discovered" first - for example, an application should be able to provide an SCI that initialized a WebSocket or JSP implementation separate from the container's and which, because it would be first in delegation order, would take precedence over the container's.
> 
> I agree 3.2 should clarify the ordering requirements for SCIs even if it just explicitly states there are no ordering guarantees or that they will be fired in the order discovered through the classloader. Until then though it should be on applications to code defensively on the assumption they may be fired in any order.


Re: SCI discovery ordering

Posted by Jeremy Boynes <jb...@apache.org>.
On Sep 19, 2013, at 8:36 AM, Mark Thomas <ma...@apache.org> wrote:

> On 19/09/2013 16:31, Jeremy Boynes wrote:
>> On Sep 19, 2013, at 6:07 AM, markt@apache.org wrote:
>> 
>>> Author: markt Date: Thu Sep 19 13:07:02 2013 New Revision:
>>> 1524727
>>> 
>>> URL: http://svn.apache.org/r1524727 Log: Always load container
>>> SCIs first
>> 
>> This seems at odds with the requirement that "the order in which
>> these services are discovered MUST follow the application’s class
>> loading delegation model" which is independent of orderedLibs. If
>> we base this on ServiceLoader, which seems to be the intent, the
>> algorithm would be:
> 
> Nope. See the discussion on the users list.
> 
> There is a clear need for container defined SCIs to be executed first
> and there is nothing (yet) in the Servlet spec that places any
> constraints on SCI execution order.

There's a clear model already without assumptions about SCI ordering: SCIs fire before SCLs, and SCLs can be ordered explicitly by web.xml. There are no ordering requirements for SCIs except that they are discovered per the delegation model. This change is changing that to load "container" SCIs firsts *even if the delegation model specifies application first.*

The original issue was that a context attribute was not set when an SCI was run (or actually, as a application class invoked by Spring's SCI was run). As Konstantin pointed out, there is no issue here if that code is run from an SCL rather than the SCI. I argue this is an application issue rather than something the container needs to enforce in a proprietary manner. It is similar to JSP land where Jasper's SCI can only register TLD listener or JSP servlet classes with the context rather than actually instantiating them.

Further, running container SCIs first may interfere with application SCIs that would be "discovered" first - for example, an application should be able to provide an SCI that initialized a WebSocket or JSP implementation separate from the container's and which, because it would be first in delegation order, would take precedence over the container's.

I agree 3.2 should clarify the ordering requirements for SCIs even if it just explicitly states there are no ordering guarantees or that they will be fired in the order discovered through the classloader. Until then though it should be on applications to code defensively on the assumption they may be fired in any order.

Re: SCI discovery ordering

Posted by Mark Thomas <ma...@apache.org>.
On 19/09/2013 16:31, Jeremy Boynes wrote:
> On Sep 19, 2013, at 6:07 AM, markt@apache.org wrote:
> 
>> Author: markt Date: Thu Sep 19 13:07:02 2013 New Revision:
>> 1524727
>> 
>> URL: http://svn.apache.org/r1524727 Log: Always load container
>> SCIs first
> 
> This seems at odds with the requirement that "the order in which
> these services are discovered MUST follow the application’s class
> loading delegation model" which is independent of orderedLibs. If
> we base this on ServiceLoader, which seems to be the intent, the
> algorithm would be:

Nope. See the discussion on the users list.

There is a clear need for container defined SCIs to be executed first
and there is nothing (yet) in the Servlet spec that places any
constraints on SCI execution order.

Mark

> 
> if (orderedLibs != null) excludedJars = URLs of all jars in
> WEB-INF/lib that are not listed in orderedLibs for (URL configFile
> : webappLoader.getResources()) { if (!excludedJars.contains(jar
> containing configFile)) { load SCIs from entries in configFile } }
> 
> I originally shied away from trying to compare the URLs returned by
> the classloader and with those from ServletContext.getResource() as
> I was was not sure they would be the same. If they are, then the
> implementation can be simplified along the lines above. If not, we
> would also need to pass in the delegation order of the classloader
> to know whether to add the "application" SCIs before or after the
> "container" ones.
> 
> I'll go ahead and modify this based on the assumption the URLs can
> be compared.
> 
> Cheers Jeremy
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org