Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use of system class loader in ClassFinder #42

Closed
jlepropre opened this issue Oct 20, 2015 · 10 comments
Closed

Use of system class loader in ClassFinder #42

jlepropre opened this issue Oct 20, 2015 · 10 comments

Comments

@jlepropre
Copy link

Hello !

First of all, a big thank for your work and framework that is very useful for us!

I have recently tried to plug Load Time Weaving (LTW) in our application using Spring and AspectJ and I faced an issue: my aspect was not weaved.

It was due to the fact that the ClassFinder loads classes in base packages using the system class loader (see org.jacpfx.rcp.util.ClassFinder.loadClass(String)). Indeed, to be weaved, the class must be loaded thanks to the Spring org.springframework.core.OverridingClassLoader which does not always delegate to the parent class loader.

So, I think that

return ClassLoader.getSystemClassLoader().loadClass(file);

should be replaced by

return this.getClass().getClassLoader().loadClass(file);

Current workaround consists in locating the aspect outside of the JacpFX base packages.

Many thanks in advance,
Cheers,
Jean.

@amoAHCP
Copy link
Collaborator

amoAHCP commented Oct 20, 2015

Hi,
thank you...
i'll change it in the 2.1 branch and will test it. I can give you a hint when the snapshot is released, it would be nice when you can test it too.
THX
Andy

@jlepropre
Copy link
Author

Hi,

Of course, I will test it!

Thanks,
Jean.

@amoAHCP
Copy link
Collaborator

amoAHCP commented Oct 21, 2015

Hi,

...
2.1-RC3

should be available on maven central.

Thanks,

Andy

@jlepropre
Copy link
Author

Hi Andy,

Ok, thanks, I will test it asap.

Cheers,
Jean.

Le 21 oct. 2015 à 21:00, Andy Moncsek notifications@github.com a écrit :

Hi,

...
2.1-RC3

should be available on maven central.

Thanks,

Andy


Reply to this email directly or view it on GitHub.

@jlepropre
Copy link
Author

Hi Andy,

I have just tested it and it does not work unfortunately: it is my mistake in fact. The issue is not the reported one but it comes from the fact that packages scanning is done before initializing the Spring context. As it is done before, classes are loaded with a class loader which has not been yet instrumented.

So in org.jacpfx.spring.launcher.AFXSpringXmlLauncher and org.jacpfx.spring.launcher.AFXSpringJavaConfigLauncher, one should replace

@SuppressWarnings("unchecked")
@Override
public void start(Stage stage) throws Exception {
    initExceptionHandler();
    scanPackegesAndInitRegestry();
    final Launcher<ClassPathXmlApplicationContext> launcher = new SpringXmlConfigLauncher(getXmlConfig());
    final Class<? extends FXWorkbench> workbenchHandler = getWorkbenchClass();
    if (workbenchHandler == null) throw new ComponentNotFoundException("no FXWorkbench class defined");
    initWorkbench(stage, launcher, workbenchHandler);
}

by

@SuppressWarnings("unchecked")
@Override
public void start(Stage stage) throws Exception {
    initExceptionHandler();
    final Launcher<ClassPathXmlApplicationContext> launcher = new SpringXmlConfigLauncher(getXmlConfig());
    scanPackegesAndInitRegestry();
    final Class<? extends FXWorkbench> workbenchHandler = getWorkbenchClass();
    if (workbenchHandler == null) throw new ComponentNotFoundException("no FXWorkbench class defined");
    initWorkbench(stage, launcher, workbenchHandler);
}

and

@SuppressWarnings("unchecked")
@Override
public void start(Stage stage) throws Exception {
    initExceptionHandler();
    scanPackegesAndInitRegestry();
    final Launcher<AnnotationConfigApplicationContext> launcher = new SpringJavaConfigLauncher(getConfigClasses());
    final Class<? extends FXWorkbench> workbenchHandler = getWorkbenchClass();
    if (workbenchHandler == null) throw new ComponentNotFoundException("no FXWorkbench class defined");
    initWorkbench(stage, launcher, workbenchHandler);
}

by

@SuppressWarnings("unchecked")
@Override
public void start(Stage stage) throws Exception {
    initExceptionHandler();
    final Launcher<AnnotationConfigApplicationContext> launcher = new SpringJavaConfigLauncher(getConfigClasses());
    scanPackegesAndInitRegestry();
    final Class<? extends FXWorkbench> workbenchHandler = getWorkbenchClass();
    if (workbenchHandler == null) throw new ComponentNotFoundException("no FXWorkbench class defined");
    initWorkbench(stage, launcher, workbenchHandler);
}

I am really sorry for that mistake.

Cheers,
Jean.

@JacpFX
Copy link
Owner

JacpFX commented Oct 22, 2015

Hi,
I’ll check it later today and give you a hint when I published a new version… from my side it should be ok to switch class loading and spring initialization.

Andy

Am 22.10.2015 um 11:09 schrieb jlepropre notifications@github.com:

Hi Andy,

I have just tested it and it does not work unfortunately: it is my mistake in fact. The issue is not the reported one but it comes from the fact that packages scanning is done before initializing the Spring context. As it is done before, classes are loaded with a class loader which has not been yet instrumented.

So in org.jacpfx.spring.launcher.AFXSpringXmlLauncher and org.jacpfx.spring.launcher.AFXSpringJavaConfigLauncher, one should replace

@SuppressWarnings("unchecked")
@OverRide
public void start(Stage stage) throws Exception {
initExceptionHandler();
scanPackegesAndInitRegestry();
final Launcher launcher = new SpringXmlConfigLauncher(getXmlConfig());
final Class<? extends FXWorkbench> workbenchHandler = getWorkbenchClass();
if (workbenchHandler == null) throw new ComponentNotFoundException("no FXWorkbench class defined");
initWorkbench(stage, launcher, workbenchHandler);
}
by

@SuppressWarnings("unchecked")
@OverRide
public void start(Stage stage) throws Exception {
initExceptionHandler();
final Launcher launcher = new SpringXmlConfigLauncher(getXmlConfig());
scanPackegesAndInitRegestry();
final Class<? extends FXWorkbench> workbenchHandler = getWorkbenchClass();
if (workbenchHandler == null) throw new ComponentNotFoundException("no FXWorkbench class defined");
initWorkbench(stage, launcher, workbenchHandler);
}
and

@SuppressWarnings("unchecked")
@OverRide
public void start(Stage stage) throws Exception {
initExceptionHandler();
scanPackegesAndInitRegestry();
final Launcher launcher = new SpringJavaConfigLauncher(getConfigClasses());
final Class<? extends FXWorkbench> workbenchHandler = getWorkbenchClass();
if (workbenchHandler == null) throw new ComponentNotFoundException("no FXWorkbench class defined");
initWorkbench(stage, launcher, workbenchHandler);
}
by

@SuppressWarnings("unchecked")
@OverRide
public void start(Stage stage) throws Exception {
initExceptionHandler();
final Launcher launcher = new SpringJavaConfigLauncher(getConfigClasses());
scanPackegesAndInitRegestry();
final Class<? extends FXWorkbench> workbenchHandler = getWorkbenchClass();
if (workbenchHandler == null) throw new ComponentNotFoundException("no FXWorkbench class defined");
initWorkbench(stage, launcher, workbenchHandler);
}
I am really sorry for that mistake.

Cheers,
Jean.


Reply to this email directly or view it on GitHub #42 (comment).

@jlepropre
Copy link
Author

Hi Andy,

Can I ask you if you got any chance to perform the change?

Thanks in advance for your answer!

Best regards,
Jean.

@amoAHCP
Copy link
Collaborator

amoAHCP commented Oct 29, 2015

Hi, sorry I am still at J1. I’ll try to make either today at the hackergarden or in the airplane tomorrow.

Andy

Am 29.10.2015 um 02:49 schrieb jlepropre notifications@github.com:

Hi Andy,

Can I ask you if you got any chance to perform the change?

Thanks in advance for your answer!

Best regards,
Jean.


Reply to this email directly or view it on GitHub #42 (comment).

@amoAHCP
Copy link
Collaborator

amoAHCP commented Oct 29, 2015

Hi,
currently we have "return this.getClass().getClassLoader().loadClass(file);" .. in JacpFX3 it will change because I'll start to prepare it for Java9. Since a Java9 module has it's on classloader (perfect for me) this won't work. Be careful with this!

JacpFX added a commit that referenced this issue Oct 29, 2015
JacpFX added a commit that referenced this issue Oct 29, 2015
JacpFX added a commit that referenced this issue Oct 29, 2015
@jlepropre
Copy link
Author

Fixed by 2.1-RC4.

Thank you very much!

Cheers,
Jean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants