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

CtClass.detach() may generate NullPointerException #258

Open
git-blame opened this issue Apr 17, 2019 · 5 comments
Open

CtClass.detach() may generate NullPointerException #258

git-blame opened this issue Apr 17, 2019 · 5 comments

Comments

@git-blame
Copy link

Here is a stack trace:

java.lang.NullPointerException
        at java.util.Hashtable.put(Hashtable.java:460)
        at javassist.ClassPool.cacheCtClass(ClassPool.java:228)
        at javassist.CtClass.detach(CtClass.java:1419)

This seems to be the conditions:

  • The detached class is not actually cached, the call to removeCached returns null leading to
  • A call to cache a null value because obj(null) != this, which HashTable (for some Java implementation) does not allow.

CtClass.java:

 public void detach() {
        ClassPool cp = getClassPool();
        CtClass obj = cp.removeCached(getName());
        if (obj != this)
            cp.cacheCtClass(getName(), obj, false);
}

HashTable.java (openjdk):

public synchronized V put(K key, V value) {

        // Make sure the value is not null

        if (value == null) {

            throw new NullPointerException();

        }

This doesn't always happen, but when it does, it seems to only happen for CtClass instances representing interfaces.

For background, I'm instrumenting classes if they implement certain interfaces. So I'm examining a class' interfaces. After which, I'm trying to be efficient by cleaning up the class pool by calling detach(). This works fine for classes but for some interfaces, it fails. Note that I'm using javassist inside a java agent that is loaded by the Boot class loader (the usual default for java agent is System class loader).

@Raymond-Naseef
Copy link

I may have time to look into this. Can you specify:

  1. the branch/version this is happening on
  2. What line in CtClass.java is at javassist.CtClass.detach(CtClass.java:1419)?cp.cacheCtClass(getName(), obj, false);?
  3. Do you have any test code for this, or any further details how to reproduce it?

@git-blame
Copy link
Author

  1. Javassist version is 3.24.0-GA.
  2. I have no idea, my version of javassist from maven does not have source file. But it is 99% likely to be cp.cacheCtClass(getName(), obj, false);
  3. The proximate cause is clear from static analysis.
public void detach() {
        ClassPool cp = getClassPool();
        CtClass obj = cp.removeCached(getName());
        if (obj != this)
            cp.cacheCtClass(getName(), obj, false);
}

removeCached simply calls HashTable.remove(). This method can return null (if key is not in hash). In this case, the class name. This makes if (obj != this) incorrectly evaluates to true resulting in a call to cache a NULL class object. The basic fix would be something like if (obj != null && obj != this)

The more interesting question is why is this CtClass (error always with a Java interface) not in the classpool?

In any case, I have attached test code here: javassist-bug.zip. Essentially, it looks at every class loaded and lists all its interfaces and superclasses. The java agent can be used when running Apache's tika parser.

  • mvn -DskipTests -PbuildAnalyzer package
  • java -javaagent:target/analyze-1.0-jar-with-dependencies.jar -jar tika-app-1.20.jar --config=tika.xml -d pom.xml
  • Example error. Note, the class type doesn't seem to matter. It is more the case that the java agent is getting its' interfaces.
[main] ERROR com.example.AnalyzerClassFileTransformer - Unknown exception org/apache/tika/mime/MimeTypes: {}
java.lang.NullPointerException
	at java.util.Hashtable.put(Hashtable.java:460)
	at javassist.ClassPool.cacheCtClass(ClassPool.java:228)
	at javassist.CtClass.detach(CtClass.java:1419)
	at com.example.AnalyzerClassFileTransformer.matchType(AnalyzerClassFileTransformer.java:46)
	at com.example.AnalyzerClassFileTransformer.transform(AnalyzerClassFileTransformer.java:91)
	at sun.instrument.TransformerManager.transform(TransformerManager.java:188)
	at sun.instrument.InstrumentationImpl.transform(InstrumentationImpl.java:428)

@alfredxiao
Copy link

I am seeing the same issue with version 3.25.0-GA.
The logic inside detach is

CtClass obj = cp.removeCached(this.getName());
        if (obj != this) {
            cp.cacheCtClass(this.getName(), obj, false);
        }

I am not sure if removeCached should always return a non-null object. If this is not the case, then a null check should be done as part of the guarding condition of the 'if' statement.

@chibash
Copy link
Member

chibash commented May 3, 2020

The specification is that any valid CtClass objects are contained in that HashMap. So removeCached should always return non null.

However, detach is a dangerous method. Once it is called, all calls to that CtClass instance would show unexpected behavior. The invariant on the HashMap would be broken. Calling detach twice raises an NullPointerException because removeCached returns null.

@SACHDEVHITESH
Copy link

I am still getting the null pointer issue in the version 3.27.0-GA

Caused by: java.lang.NullPointerException
at javassist.ClassPool.cacheCtClass(ClassPool.java:236)
at javassist.CtClass.detach(CtClass.java:1427)
at com.microsoft.intune.mam.TransformationUnit.writeClassesToDirectory(TransformationUnit.java:275)
at com.microsoft.intune.mam.TransformationUnit.writeOutput(TransformationUnit.java:126)
at com.microsoft.intune.mam.BuildTimeMamifier.mamify(BuildTimeMamifier.java:131)
at com.microsoft.intune.mam.MamifyTransformBase.transform(MamifyTransformBase.java:195)
at com.android.build.gradle.internal.pipeline.TransformTask$2.call(TransformTask.java:284)
at com.android.builder.profile.ThreadRecorder.record(ThreadRecorder.java:69)
... 126 more

New issue Link
https://github.com/jboss-javassist/javassist/issues/442

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

5 participants