2015-04-22 0:01 GMT+02:00 Hendrik Dev <[email protected]>:
> On Tue, Apr 21, 2015 at 11:06 PM, Romain Manni-Bucau
> <[email protected]> wrote:
> > Le 21 avr. 2015 22:51, "Hendrik Dev" <[email protected]> a écrit :
> >>
> >> A few thoughts and questions on
> >>
> >> JsonProvider.doLoadProvider():
> >> - "tccl" can be null (in case of system classloader) but thats never
> >> really checked
> >
> > If so johnzon cant be loaded isnt it? So not a big deal imo
>
> someone could set the context classloader explicitly to null, see attached
> diff
>
>
Hmm, thought it was already working thanks to cl system call but nothing
against this part of the patch.
> >
> >> - special handling org.apache.geronimo.osgi.locator.ProviderLocator
> >> really needed here?
> >>
> >
> > In G spec jars yes.
>
> OK
>
>
In your patch you "forName" it in a static fashion, this shouldnt be the
case to support overriding + OSGi correctly. A side note I completely
missed too: should use tccl.loadClass and not Class.forName cause of OSGi.
> >
> >> JsonProvider.provider():
> >> - doPrivileged/SecurityManager check really needed here?
> >
> > For containers yes and doesnt hurt at runtime normally.
>
> OK
>
> >
> >> - method seems thread safe but we do not cache the returned provider
> >> instance. Maybe we can to this in a thread local variable?
> >>
> >
> > Not cached for container case + i dont expect it to be called often.
> >
> > Thread local would break ears or wars if johnzon is in one war, jackson
> in
> > another and api in the container for instance + it would leak on
> undeploy.
>
> i see, so maybe caching the hole provider, see attached diff
>
>
-1, was my first impl but if you have this deployment (tree classloader but
OSGi would make it worse):
container loader
`- geronimo-json
|- webapp 1
| `- johnzon-0.4
`- webapp 2
`- johnzon-0.7
Then you would leak the provider (impl) in the container.
Now think to OSGi...headache ;)
> >
> > Did you hit any issue?
>
> nothing special, originally working on that to make sure that Johnzon
> is working under OSGi (Karaf), so i looked at the RI and johnzon
> specs.
>
> >
> >> Thanks
> >> Hendrik
> >>
> >> --
> >> Hendrik Saly (salyh, hendrikdev22)
> >> @hendrikdev22
> >> PGP: 0x22D7F6EC
>
>
>
> --
> Hendrik Saly (salyh, hendrikdev22)
> @hendrikdev22
> PGP: 0x22D7F6EC
>
