Patch: issue RT-14177

[Florian Brunner]

Hi Tom,

I have the following bundles:

Bundle A:
SomeBaseControl with a custom skin in a different (most commonly private) package

Bundle B
SomeControl extends SomeBaseControl 

Bundle C
Instantiates SomeControl

Just using: 

getClass().getClassLoader().loadClass(className);

in Control resulted in a ClassNotFoundException (here: getClass() == SomeControl.class; className=skin class name for SomeBaseControl).
So I started to loop over the super-types to get a class which is in the same bundle as the custom skin class. That worked.

Do you see a better way without changing the API?

Regards,
Florian


Am Freitag 01 Juni 2012, 16:44:24 schrieb Tom Schindl:
> Hi Florian,
> 
> Good. I can you tell me why the super-type loop is used - I think it was
> already in your first patch - wondering what this is good for.
> 
> Tom
> 
> Am 01.06.12 16:38, schrieb Florian Brunner:
> > 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
> >>>>>>>>>>>>>
> >>>>>>>
> >>
> >>
> >>
> 
> 
>