pushed a fix reverting the revert ;)

changes are

1. we take the classloader from the servlet context instead of the tccl ->
should be noop but way more secured
2. if there is no default bus we set one, avoids the server one to ends up
being a default classloader and leaking, we also reset it if we set it

hope it should fix most of the issues


Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>

2018-02-17 16:48 GMT+01:00 Romain Manni-Bucau <[email protected]>:

> Can you share the 2 locking threads plz?
>
> Le 17 févr. 2018 16:41, "Romain Manni-Bucau" <[email protected]> a
> écrit :
>
>> What do you run? Always green here :(
>>
>> Le 17 févr. 2018 11:51, "Mark Struberg" <[email protected]> a
>> écrit :
>>
>>> Yes, been deterministic. At my own box but also on Reinhards. Broken
>>> 100% of all runs.
>>>
>>> LieGrue,
>>> strub
>>>
>>>
>>> > Am 17.02.2018 um 11:16 schrieb Romain Manni-Bucau <
>>> [email protected]>:
>>> >
>>> > Le 17 févr. 2018 10:09, "Mark Struberg" <[email protected]> a
>>> > écrit :
>>> >
>>> > In the test there are 2 classloaders involved.
>>> > a.) the client side
>>> > b.) the server (tomcat).
>>> >
>>> > with the setExtension using the current TCCL we kind of leak the
>>> Tomcat CL
>>> > into the client it seems.
>>> > It blows up with the Tomcat ParallelWebAppClassloader already not being
>>> > active anymore.
>>> > And the location where this happens is on the client side
>>> (transforming the
>>> > attachment FROM jsonb to object).
>>> >
>>> > i've also seen random CL errors in our real world project in the last
>>> week.
>>> > Might have something to do as well...
>>> >
>>> >
>>> > In webapp case then cause in classpath mode it should be the same.
>>> >
>>> > Issue is more the client inherit from the default bus instead of using
>>> its
>>> > own bus but it always has been this way. Isnt it a side effect of
>>> creating
>>> > a classloader to run tests in parallel?
>>> >
>>> > Are you able to reproduce it deterministicly?
>>> >
>>> >
>>> >
>>> > LieGrue,
>>> > strub
>>> >
>>> >> Am 17.02.2018 um 09:41 schrieb Romain Manni-Bucau <
>>> [email protected]>:
>>> >>
>>> >> Le 16 févr. 2018 22:07, "Mark Struberg" <[email protected]> a
>>> >> écrit :
>>> >>
>>> >> Hi Romain!
>>> >>
>>> >> Reinhard managed to reproduce the issue when running the latest MW
>>> source
>>> >> on his box.
>>> >>
>>> >> With git-bisect we figured it has to do with r1822816
>>> >>
>>> >> MeecrowaveBus
>>> >> "simplifying MeecrowaveBus and enforcing the classloader to avoid to
>>> rely
>>> >> on resource in ServletController when unneeded"
>>> >>
>>> >> The main change is to move the TCCL resolution and calling
>>> setExtension.
>>> >> This seems to break the Tomcat ParallelWebAppClassLoader.
>>> >> Rather it seems that the cleanup then is being performed on the tomcat
>>> >> side, even if the server is already shut down.
>>> >>
>>> >> I gonna revert this change for now to make all tests pass again.
>>> >> But tbh I have no clue what setExtension does in detail so plz review
>>> it.
>>> >>
>>> >>
>>> >>
>>> >> This is the classloader usee by the cxf deployment everywhere. If not
>>> det
>>> >> it is trivial not have a broken setup during all the other loading and
>>> >> deployment.
>>> >>
>>> >> How does it lock parallel loading? I cant reproduce it.
>>> >>
>>> >>
>>> >>
>>> >>
>>> >>
>>> >>
>>> >> LieGrue,
>>> >> strub & Reinhard
>>> >>
>>> >>
>>> >>> Am 16.02.2018 um 15:42 schrieb Mark Struberg
>>> <[email protected]>:
>>> >>>
>>> >>> -DreuseForks=false indeed lets the problem go away.
>>> >>>
>>> >>> Now we need to find out where this side effect is coming from.
>>> >>>
>>> >>> LieGrue,
>>> >>> strub
>>> >>>
>>> >>>> Am 16.02.2018 um 14:31 schrieb Romain Manni-Bucau <
>>> [email protected]
>>> >>> :
>>> >>>>
>>> >>>> Hi Mark, can't reproduce it but used a johnzon 1.1.6 created from
>>> the
>>> >>>> 1.1.7-SNAPSHOT since the tag is not pushed on asf yet. Not sure it
>>> has
>>> > an
>>> >>>> impact or if it is the test order which is responsible of it.Maybe
>>> try
>>> >>>> forcing reuseFork=false in surefire to validate this hypothesis
>>> >>>>
>>> >>>>
>>> >>>> Romain Manni-Bucau
>>> >>>> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>>> >>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>> >>>> <http://rmannibucau.wordpress.com> | Github <https://github.com/
>>> >> rmannibucau> |
>>> >>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>>> >>>> <https://www.packtpub.com/application-development/java-
>>> >> ee-8-high-performance>
>>> >>>>
>>> >>>> 2018-02-16 13:56 GMT+01:00 Mark Struberg <[email protected]
>>> >:
>>> >>>>
>>> >>>>> no worries, we first have to fix a seemingly random test failure
>>> in MW
>>> >>>>> anyway
>>> >>>>>
>>> >>>>> Tests in error:
>>> >>>>> MultipartTest.configBinding:51 » ResponseProcessing Problem with
>>> > reading
>>> >>>>> the d...
>>> >>>>>
>>> >>>>> happens only when this test is executed with all other tests.
>>> >>>>> Wenn running the test alone all is green.
>>> >>>>>
>>> >>>>> Anyone want's to take that up?
>>> >>>>> I'm offline for the next few hours now.
>>> >>>>>
>>> >>>>> LieGrue,
>>> >>>>> strub
>>> >>>>>
>>> >>>>>
>>> >>>>>> Am 16.02.2018 um 13:49 schrieb Romain Manni-Bucau <
>>> >> [email protected]
>>> >>>>>> :
>>> >>>>>>
>>> >>>>>> +1
>>> >>>>>>
>>> >>>>>> Side note: can you ensure it is either not deployed or staging
>>> repo
>>> > are
>>> >>>>> in
>>> >>>>>> the pom to not break early consumers please?
>>> >>>>>>
>>> >>>>>>
>>> >>>>>> Le 16 févr. 2018 13:19, "Mark Struberg" <[email protected]>
>>> a
>>> >>>>>> écrit :
>>> >>>>>>
>>> >>>>>>> will roll a MW release in the afternoon.
>>> >>>>>>>
>>> >>>>>>> Currently testing the updates to the staged versions.
>>> >>>>>>>
>>> >>>>>>> LieGrue,
>>> >>>>>>> strub
>>>
>>>

Reply via email to