javassist icon indicating copy to clipboard operation
javassist copied to clipboard

CtClass.detach() may generate NullPointerException

Open git-blame opened this issue 5 years ago • 5 comments

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).

git-blame avatar Apr 17 '19 11:04 git-blame

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?

Raymond-Naseef avatar Apr 17 '19 15:04 Raymond-Naseef

  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)

git-blame avatar Apr 22 '19 14:04 git-blame

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.

alfredxiao avatar May 03 '20 02:05 alfredxiao

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.

chibash avatar May 03 '20 05:05 chibash

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

SACHDEVHITESH avatar Jan 19 '23 11:01 SACHDEVHITESH