Patch: issue RT-14177

Fri Jun 1 07:38:06 PDT 2012

I tested rt-14177.3.patch and Tom is right, the ClassNotFoundException has to be catched otherwise the new code won't be executed.

Unfortunatly, hg import failed for 5.patch for some reason.

I created and tested a new patch (with "hg diff --git" as recommended by Kevin (see attachements, I don't have the necessary rights in JIRA).

It works fine for my use case.

I also renamed ex2 to ex3 and ex3 to ex2 and fixed some formatting issues.

Note however that now the method contains around 100 lines of code. You might want to refactor it into several smaller methods for maintainability reasons.

Regards,
Florian

Am Freitag 01 Juni 2012, 11:04:09 schrieb Tom Schindl:
> Hi,
> 
> I've uploaded a fixed version of the patch! I think Richards patch
> missed to catch the CNE for the context classloader.
> 
> I can not comment on other areas of the control system - just did a
> short Class.forName-scan and can confirm you are finding about
> PopControl - it even has the same fix for RT-17525 so extracing the code
> and moveing it to a library makes sense.
> 
> CSS might have similar problems (but in case of classloading but they
> are harder to fix because one has to infer the correct classloader from
> the CSS-URI or even the target they are applied to!).
> 
> FXML has fixed all iusses (beside one small remaining issue I'm just
> discussing with Greg on this list) because it allows to pass custom
> classloaders and hence stuff loaded through FXML will directly profit
> from the Control enhancement.
> 
> FXML is IMHO different to CSS/Controls because there's a defined
> lifecycle and so controlling the classloader there is much easier than
> it is in CSS/Control space where the classloader access is happening in
> an async fashion.
> 
> I currently have no local build so I have to rely on Florian to give
> feedback if it fixes the problem. From a source introspection it looks
> good to me with my patch 5.
> 
> Tom
> 
> Am 01.06.12 05:22, schrieb Jonathan Giles:
> > +1 on the patch, although I do wonder whether we are being consistent in
> > the other places where classloading comes into play. Is this something
> > that should be extracted and used as an internal library?
> > 
> > For example, there is much the same code in PopupControl that needs to
> > be fixed at the same time as the patch you are submitting here, but more
> > abstractly, should we have a library rather than duplicate similar code
> > in FXML and CSS as well?
> > 
> > -- Jonathan
> > 
> > 
> > On 1/06/2012 3:14 p.m., Richard Bair wrote:
> >> Florian / Tom,
> >>
> >> I've uploaded two patches to the issue:
> >> rt-14177.2.patch
> >> rt-14177.3.patch
> >>
> >> The .3 patch is my preferred one. Can you take a look / apply it and
> >> see if it solves your issue? If the .3 patch works, then I think we've
> >> got a clean patch which will produce no additional performance
> >> overhead to a correctly functioning JavaFX application today, but will
> >> fix your issue (at least as Tom said in the issue itself for 95% of
> >> the folks).
> >>
> >> We can then file a new issue for the remaining 5% cases which will
> >> need more time (since it involves some form of new API) whereas in
> >> this case it should "just work".
> >>
> >> David / Jonathan, can you guys give a quick code review? I'm pretty
> >> sure the code does what I want it to do, and hopefully Florian / Tom
> >> will tell us if what I want it to do is actually the right thing to do
> >> (ie: solves the issue) :-).
> >>
> >> Thanks
> >> Richard
> >>
> >> On May 31, 2012, at 5:45 PM, Richard Bair wrote:
> >>
> >>> Hi Florian,
> >>>
> >>> We'll get this fixed ASAP.
> >>>
> >>> Thanks
> >>> Richard
> >>>
> >>> On May 31, 2012, at 4:45 PM, Kevin Rushforth wrote:
> >>>
> >>>> Hi Florian,
> >>>>
> >>>> This JIRA issue is targeted for 3.0 which is just in the initial
> >>>> planning stages, so no one has looked at it yet.
> >>>>
> >>>> The patch as attached to the issue deals with one case of CSS
> >>>> loading a skin, but when the controls team evaluated it, their take
> >>>> was that it a more general platform issue, although it could be
> >>>> dealt with in isolation. In any case, even if I were to assign this
> >>>> back to someone on the controls team, this code would need to be
> >>>> evaluated for potential bugs and to ensure that there are no
> >>>> security issues (probably not an issue, but since it would be using
> >>>> a system class loader we need to make sure it is never called from a
> >>>> privileged context), which is unlikely to happen for the current
> >>>> release.
> >>>>
> >>>> -- Kevin
> >>>>
> >>>>
> >>>> Florian Brunner wrote:
> >>>>> Hi Kevin,
> >>>>>
> >>>>> What is the status of my patch?
> >>>>> As mentioned in JIRA, this issue is becoming a blocker! Please
> >>>>> increase the priority! (Nobody from Oracle is responding on JIRA).
> >>>>>
> >>>>> Can we agree then to integrate the provided patch? It improves the
> >>>>> situation but doesn't introduce any API additions and we can
> >>>>> replace/ complete it later with other solutions if needed.
> >>>>> Please tell me if I should change something in the patch to get it
> >>>>> approved.
> >>>>>
> >>>>> Regards,
> >>>>> Florian
> >>>>>
> >>>>> Am Freitag 06 April 2012, 23:58:13 schrieb Kevin Rushforth:
> >>>>>
> >>>>>> I attached it to the JIRA issue.
> >>>>>>
> >>>>>> -- Kevin
> >>>>>>
> >>>>>>
> >>>>>> Florian Brunner wrote:
> >>>>>>
> >>>>>>> Here the patch created with "hg diff --git" and zipped.
> >>>>>>>
> >>>>>>> Regards,
> >>>>>>> Florian
> >>>>>>>
> >>>>>>> Am Freitag 06 April 2012, 18:31:23 schrieb Kevin Rushforth:
> >>>>>>>
> >>>>>>>> Right. Since you didn't file the issue and are not the assignee,
> >>>>>>>> you can't add an attachment.
> >>>>>>>>
> >>>>>>>> Copying Brian Beck in case this is something we can (and want
> >>>>>>>> to) fix.
> >>>>>>>>
> >>>>>>>> In the mean time, you can e-mail me the patch and I'll attach it
> >>>>>>>> to the bug report.
> >>>>>>>>
> >>>>>>>> -- Kevin
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Florian Brunner wrote:
> >>>>>>>>
> >>>>>>>>> Hi Kevin,
> >>>>>>>>>
> >>>>>>>>> Thanks for your response.
> >>>>>>>>>
> >>>>>>>>> Unfortunatly, I don't see a way to attach something to the JIRA
> >>>>>>>>> issue.
> >>>>>>>>>
> >>>>>>>>> Either I'm missing something or maybe I need additional rights?
> >>>>>>>>>
> >>>>>>>>> Regards,
> >>>>>>>>> Florian
> >>>>>>>>>
> >>>>>>>>> Am Donnerstag 05 April 2012, 18:54:38 schrieben Sie:
> >>>>>>>>>
> >>>>>>>>>> Great. Please attach the patch as a zip file to the JIRA issue
> >>>>>>>>>> (preferably generated with either webrev or "hg diff --git")
> >>>>>>>>>> so that it can be evaluated.
> >>>>>>>>>>
> >>>>>>>>>> -- Kevin
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Florian Brunner wrote:
> >>>>>>>>>>
> >>>>>>>>>>> Hi,
> >>>>>>>>>>>
> >>>>>>>>>>> Let me introduce myself: my name is Florian Brunner and I'm a
> >>>>>>>>>>> Senior Software Engineer from Switzerland.
> >>>>>>>>>>>
> >>>>>>>>>>> I started to use JavaFX in an OSGi environment. Now I
> >>>>>>>>>>> stumbled over the following issue, which blocks me:
> >>>>>>>>>>> http://javafx-jira.kenai.com/browse/RT-14177
> >>>>>>>>>>>
> >>>>>>>>>>> I have written a patch now. It probably doesn't solve the
> >>>>>>>>>>> whole issue, but at least it unblocks me for now.
> >>>>>>>>>>>
> >>>>>>>>>>> It would be great if you could integrate it.
> >>>>>>>>>>>
> >>>>>>>>>>> Note that I contributed to OpenJDK before (Swing) and have
> >>>>>>>>>>> signed the Sun Contributor Agreement:
> >>>>>>>>>>>
> >>>>>>>>>>> http://www.oracle.com/technetwork/community/oca-486395.html#b
> >>>>>>>>>>> Florian Brunner - java.net - puce
> >>>>>>>>>>>
> >>>>>>>>>>> Regards,
> >>>>>>>>>>> Florian
> >>>>>>>>>>>
> >>>>>
> 
> 
> 