ok, but just wondering why the RI does threadlocal caching if its not working within containers
On Wed, Apr 22, 2015 at 9:06 AM, Romain Manni-Bucau <[email protected]> wrote: > 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 >> -- Hendrik Saly (salyh, hendrikdev22) @hendrikdev22 PGP: 0x22D7F6EC
