fix: more resilient wrt to errors during shadow class building #3655

BuzzMS · 2020-10-16T10:17:45Z

Hello everyone,

We develop a static code analyzer and to do it we need to build models using Spoon for a lot of projects.

Some background information.

This is how it was before:

JDTImportBuilder::getOrLoadClass tried to get imported class through getClass().getClassLoader().loadClass(className) ... it didn't manage to find anything ('null', ClassNotFoundException, ...).
This led to the situation when JDTImportBuilder::getOrLoadClass returned 'null'. We couldn't get it and that was OK.

This is how it is now:

Recently JDTImportBuilder::getOrLoadClass tries to load class using JDTImportBuilder::loadClass.
That is, if 'factory.getEnvironment().getInputClassLoader' existed, we tried to get it from there, otherwise we got class, same as before.
So through the factory environment you are able to get classes that, before the change, thrown with ClassNotFoundException.
But this modification led to the situation when TypeFactory::get(Class) couldn't get the shadowClass using JavaReflectionTreeBuilder (probably because reflection has not enough information). This leads to the SpoonClassNotFoundException exception that interrupts model building. Therefore we can't start the analysis niether.
There are a lot of reasons of program crash when launching JavaReflectionTreeBuilder:

    * Caused by: java.lang.ExceptionInInitializerError
    * Caused by: java.lang.TypeNotPresentException: Type org.jvnet.mimepull.Header not present
    * Caused by: java.lang.ArrayStoreException: sun.reflect.annotation.TypeNotPresentExceptionProxy
    * Caused by: java.lang.SecurityException: class "com.google.protobuf.GeneratedMessageV3$FieldAccessorTable"'s signer information does not match signer information of other classes in the same package
    * Caused by: java.lang.IncompatibleClassChangeError: com.google.common.collect.Interners and com.google.common.collect.Interners$InternerImpl disagree on InnerClasses attribute
    * ....

PR details:
I added logging 'cannot create shadow class' (just in case).
I think, returning 'null' makes more sense, as it was before. If you couldn't get it, that's OK (alas, something's missing).

…ging.

MartinWitt · 2020-10-16T10:58:25Z

With the recent change you mean #3454 or? Could you provide an example where a shadow class can't be built?
An class that isn't on the spoon path and cant be build is an unnormal state. Returning null would hide this error and not solve the problem. Null is and was never meant as error state.

BuzzMS · 2020-10-16T12:43:12Z

I can't reproduce it on the test yet =((

I'll show you one of the examples we already have.

Let's have a look at the 'dspace' project.
Before the analysis, we gather information on the source file for every module as well as classpath.
Then we launch buildModel with all the information we have.

And we get the following:


[INFO] [ 52%] Analyze module dspace-xmlui
[INFO] Error: cannot create shadow class: org.dspace.harvest.HarvestScheduler
[INFO] spoon.support.SpoonClassNotFoundException: cannot create shadow class: org.dspace.harvest.HarvestScheduler
[INFO] 	at spoon.reflect.factory.TypeFactory.get(TypeFactory.java:566)
[INFO] 	at spoon.support.compiler.jdt.JDTImportBuilder.getOrLoadClass(JDTImportBuilder.java:153)
[INFO] 	at spoon.support.compiler.jdt.JDTImportBuilder.build(JDTImportBuilder.java:78)
[INFO] 	at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.lambda$buildModel$1(JDTBasedSpoonCompiler.java:441)
[INFO] 	at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.forEachCompilationUnit(JDTBasedSpoonCompiler.java:466)
[INFO] 	at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.buildModel(JDTBasedSpoonCompiler.java:440)
[INFO] 	at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.buildUnitsAndModel(JDTBasedSpoonCompiler.java:372)
[INFO] 	at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.buildSources(JDTBasedSpoonCompiler.java:337)
[INFO] 	at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.build(JDTBasedSpoonCompiler.java:114)
[INFO] 	at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.build(JDTBasedSpoonCompiler.java:97)
[INFO] 	at spoon.Launcher.buildModel(Launcher.java:755)
[INFO] 	at com.pvsstudio.projects.Module.build(Module.java:336)
[INFO] 	at com.pvsstudio.runner.ModuleWorker.run(ModuleWorker.java:49)
[INFO] 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
[INFO] 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
[INFO] 	at java.lang.Thread.run(Thread.java:748)
[INFO] Caused by: java.lang.ExceptionInInitializerError
[INFO] 	at sun.misc.Unsafe.ensureClassInitialized(Native Method)
[INFO] 	at sun.reflect.UnsafeFieldAccessorFactory.newFieldAccessor(UnsafeFieldAccessorFactory.java:43)
[INFO] 	at sun.reflect.ReflectionFactory.newFieldAccessor(ReflectionFactory.java:156)
[INFO] 	at java.lang.reflect.Field.acquireFieldAccessor(Field.java:1088)
[INFO] 	at java.lang.reflect.Field.getFieldAccessor(Field.java:1069)
[INFO] 	at java.lang.reflect.Field.get(Field.java:393)
[INFO] 	at spoon.support.visitor.java.JavaReflectionTreeBuilder.visitField(JavaReflectionTreeBuilder.java:296)
[INFO] 	at spoon.support.visitor.java.JavaReflectionVisitorImpl.visitClass(JavaReflectionVisitorImpl.java:92)
[INFO] 	at spoon.support.visitor.java.JavaReflectionTreeBuilder.visitClass(JavaReflectionTreeBuilder.java:152)
[INFO] 	at spoon.support.visitor.java.JavaReflectionTreeBuilder.scan(JavaReflectionTreeBuilder.java:109)
[INFO] 	at spoon.reflect.factory.TypeFactory.get(TypeFactory.java:564)
[INFO] 	... 15 more
[INFO] Caused by: java.lang.IllegalStateException: DSpace kernel cannot be null
[INFO] 	at org.dspace.utils.DSpace.getServiceManager(DSpace.java:63)
[INFO] 	at org.dspace.services.factory.DSpaceServicesFactory.getInstance(DSpaceServicesFactory.java:35)
[INFO] 	at org.dspace.content.factory.ContentServiceFactory.getInstance(ContentServiceFactory.java:102)
[INFO] 	at org.dspace.harvest.HarvestScheduler.<clinit>(HarvestScheduler.java:76)
[INFO] 	... 26 more

When improting class, Spoon tries to intialize static context for this class as if code was launched with JVM at the 'dspace' app startup.
But some field was 'null' while doing static initialization.

This indicates the following:

[INFO] Caused by: java.lang.IllegalStateException: DSpace kernel cannot be null

Eventually, while doing static initialization, IllegalStateException arises -> thus ExceptionInInitializerError does as well.
There are a lot of reasons of an emergency situation because there is lack of information.

BuzzMS · 2020-10-20T07:23:44Z

Added a test. At windows 10 I have a test pass. Not in CI. Sort it out.

BuzzMS · 2020-10-20T09:01:53Z

I added test that causes the problem of creating shadow class.

If our importing class contains public static primitive field, we try to get its value through Reflection API mechanism.
As a result, JVM loads the whole class (whole static context) before getting the value.
If one of the fields can cause exception, it will lead to exception (although we wanted to to get a literal value).

monperrus

Thanks a lot for the test case (it's very important, we always specify bugs with test cases in Spoon).

It's better to avoid return null(there are NPE time bombs).

What about using the null object design pattern here and returning a specific object that represents the failed class?

monperrus · 2020-10-22T05:09:56Z

src/test/java/spoon/support/visitor/java/JavaReflectionTreeBuilderTest.java

+
+ @Test
+ public void testExpectedExceptionInInitializerError() {
+ try {


could you add a comment "contract: ..." in natural language where you describe the intention of the test? Thanks

BuzzMS · 2020-10-22T14:35:14Z

Hi everyone!

In JavaReflectionTreeBuilder#visitField(Field), I extended the exceptions that we can catch (up to Throwable). This way, we won't interrupt when creating a shadow class (we won't only set defaultExpression for public static primitive)

In TypeFactory#get(Class), if by some reasons it's not possible to create a shadow class, a CtType with brief information on the package and the class name will be returned.
Can we apply the null object design pattern in this form? Or do we have to still use the entity, for example, CtTypeEmptyImpl?

In accordance with the edits, I changed the test.

src/main/java/spoon/support/visitor/java/JavaReflectionTreeBuilder.java

BuzzMS · 2020-10-23T09:21:01Z

I looked through all the projects crashes caused by java.lang.reflect.Field.get() and found out the following exceptions: ExceptionInInitializerError, UnsatisfiedLinkError.
I made the appropriate corrections and added a test.

Therefore, all the crashes for some reason will lead to logging and building stub classes in TypeFactory#get(Class)

monperrus · 2020-10-24T11:43:02Z

Thanks a lot. LGTM, will merge.

monperrus · 2020-10-26T06:11:49Z

Thanks a lot for your contribution.

Replaced throwing an exception 'SpoonClassNotFoundException' with log…

de15384

…ging.

BuzzMS closed this Oct 16, 2020

BuzzMS reopened this Oct 16, 2020

remove unused import

d9b990c

Added a test when JavaReflectionTreeBuilder throws an exception.

2e282b2

Minor fix. The test for a class and a single test are run differently.

4c8fbcf

monperrus reviewed Oct 22, 2020

View reviewed changes

Rework of exception ("cannot create shadow class")

c090ca9

monperrus reviewed Oct 23, 2020

View reviewed changes

src/main/java/spoon/support/visitor/java/JavaReflectionTreeBuilder.java Show resolved Hide resolved

Rework of exception N2 ("cannot create shadow class")

8287112

monperrus changed the title ~~Replaced throwing of 'SpoonClassNotFoundException' exception with logging (TypeFactory::get)~~ fix: more resilient wrt to errors during shadow class building Oct 26, 2020

monperrus merged commit 4ccbf0a into INRIA:master Oct 26, 2020

monperrus mentioned this pull request Mar 16, 2021

Major Release Spoon 9.0.0 #3845

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: more resilient wrt to errors during shadow class building #3655

fix: more resilient wrt to errors during shadow class building #3655

BuzzMS commented Oct 16, 2020

MartinWitt commented Oct 16, 2020

BuzzMS commented Oct 16, 2020

BuzzMS commented Oct 20, 2020

BuzzMS commented Oct 20, 2020

monperrus left a comment

monperrus Oct 22, 2020

BuzzMS Oct 22, 2020

BuzzMS commented Oct 22, 2020

BuzzMS commented Oct 23, 2020

monperrus commented Oct 24, 2020

monperrus commented Oct 26, 2020

fix: more resilient wrt to errors during shadow class building #3655

fix: more resilient wrt to errors during shadow class building #3655

Conversation

BuzzMS commented Oct 16, 2020

MartinWitt commented Oct 16, 2020

BuzzMS commented Oct 16, 2020

BuzzMS commented Oct 20, 2020

BuzzMS commented Oct 20, 2020

monperrus left a comment

Choose a reason for hiding this comment

monperrus Oct 22, 2020

Choose a reason for hiding this comment

BuzzMS Oct 22, 2020

Choose a reason for hiding this comment

BuzzMS commented Oct 22, 2020

BuzzMS commented Oct 23, 2020

monperrus commented Oct 24, 2020

monperrus commented Oct 26, 2020