santuario-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Colm O hEigeartaigh <cohei...@apache.org>
Subject Re: Thread safety and ResourceResolverSpi
Date Tue, 31 Jul 2012 10:24:13 GMT
Hi Eric,

I agree that there is a potential issue here, although as you point out
it's unlikely to occur in practise. Could you open a JIRA?

Thanks,

Colm.

On Wed, Jul 25, 2012 at 10:38 PM, Eric Johnson <eric@tibco.com> wrote:

> I've been looking over the Santuario Java code and I noticed what appears
> to be a bug.
>
> Specifically this code (with details not needed elided) from
> ResourceResolver.getInstance()**:
>
>             for (ResourceResolver resolver : resolverList) {
>                 ResourceResolver resolverTmp = resolver;
>                 if (!resolver.resolverSpi.**engineIsThreadSafe()) {
>                     // generate a new instances of resolverSpi ...
>                 }
>
>                 // ...
>
>                 resolverTmp.resolverSpi.**secureValidation =
> secureValidation;
>                 if ((resolverTmp != null) && resolverTmp.canResolve(uri,
> baseURI)) {
>                     // Check to see whether the Resolver is allowed
>                     // check for certain types ....
>                     return resolverTmp;
>                 }
>             }
>
> In case you didn't see it, the trouble is the juxtaposition of
> "resolverSpi.**engineIsThreadSafe()" followed by code that sets
> "secureValidation" on the very same instance of the spi, whether or not it
> is thread safe.
>
> Meaning, if two threads resolve at the same time, and one thread is
> attempting secure resolution while the other is not, all the "thread safe"
> resolvers risk a race condition where they will now be in an uncertain
> state. Of course, it turns out that all the built-in resolvers declare
> themselves thread-safe, so this magnifies the problem.
>
> In practice, this is not likely ever to occur, because any given
> application will likely share the same notion of "secureValidation".
>
> Am I mis-reading this code?
>
> Three observations
>
> #1 - the secureValidation flag needs only be set for the resolver that is
> actually chosen
>
> #2 - the secureValidation flag should instead be passed as a parameter
> ResourceResolverSpi.**engineResolve() method, not stored as data in the
> SPI instance.
>
> #3 - if there were ever a resolver to show up that isn't thread safe, and
> it also has properties, the logic above which creates a new instance of the
> SPI would obliterate the properties of the spi. It turns out only the HTTP
> resolver uses these properties for anything, so this may not be problematic
> in the real world.
>
> -Eric.
>



-- 
Colm O hEigeartaigh

Talend Community Coder
http://coders.talend.com

Mime
View raw message