Use of Class.getResourceAsStream() is not thread safe due to JDK bug [137929327]

Fixed

Bug

Status Update

No update yet.

Description

hu...@google.com

created issue #1

Jul 19, 2019 11:48AM

Problem
-----------
It is easy to see code written in this form: SomeClass.class.getResourceAsStream(someResource) (see

https://docs.oracle.com/javase/8/docs/api/java/lang/Class.html#getResourceAsStream-java.lang.String-). However, this code is problematic when multiple class loaders are involved in a parallel build, resulting in concurrency bugs such as

Issue 120277395

.

What's wrong with Class.getResourceAsStream()?
-------------------------------------------------------------------
Calling Class.getResourceAsStream() is roughly equivalent to calling Class.getClassLoader().getResourceAsStream().

The implementation of ClassLoader.getResourcesAsStream() is probably safe, but when the class loader is a URLClassLoader, then its overridden version URLClassLoader.getResourceAsStream() has a bug:

https://bugs.openjdk.java.net/browse/JDK-8205976. (The bug has some sample code that well explains the issue.)

This bug can manifest when two subprojects are building in parallel in separate class loaders, and both call URLClassLoader.getResourceAsStream(). One subproject is finished first, and the corresponding class loader is closed. However, closing the class loader will also close the underlying jar file that the other project might be using when reading the stream returned by URLClassLoader.getResourceAsStream(). (This is what happened in

Issue 120277395

.)

Note: Calling Class.getResourceAsStream() is okay if Class.getClassLoader() is not a URLClassLoader, and therefore does not suffer from this bug. However, because we usually don't check the ClassLoader type, it is better to simply avoid calling Class.getResourceAsStream().

What's the alternative?
------------------------------
One alternative is to use the default implementation of ClassLoader.getResourcesAsStream(), which is Class.getResource(someResource).openStream().

I've verified locally that using this alternative, closing the class loader doesn't close the underlying jar file, and it should be thread safe. (It fixes

Issue 120277395

.)

I'm opening this bug to revise the code in AGP, data binding, and Jetifier with this new understanding.

Comments

cm...@google.com <cm...@google.com> #2Jul 22, 2019 01:35PM

The issue is that Jetifier modifies jars and does not remove signatures. Below is the original class:

Classfile /tmp/non-jetified/org/bouncycastle/jce/provider/ExtCRLException.class
Last modified Mar 1, 2015; size 362 bytes
MD5 checksum 3968a716f9e60e1ad36c34157bd0239f
class org.bouncycastle.jce.provider.ExtCRLException extends java.security.cert.CRLException
minor version: 0
major version: 49
flags: ACC_SUPER
Constant pool:
#1 = Utf8 <init>
#2 = Utf8 cause
#3 = Utf8 getCause
#4 = Utf8 java/security/cert/CRLException
#5 = Utf8 org/bouncycastle/jce/provider/ExtCRLException
#6 = Class #4 // java/security/cert/CRLException
#7 = Class #5 // org/bouncycastle/jce/provider/ExtCRLException
#8 = Utf8 Ljava/lang/Throwable;
#9 = Utf8 (Ljava/lang/String;)V
#10 = Utf8 ()Ljava/lang/Throwable;
#11 = Utf8 (Ljava/lang/String;Ljava/lang/Throwable;)V
#12 = NameAndType #2:#8 // cause:Ljava/lang/Throwable;
#13 = NameAndType #1:#9 // "<init>":(Ljava/lang/String;)V
#14 = Fieldref #7.#12 // org/bouncycastle/jce/provider/ExtCRLException.cause:Ljava/lang/Throwable;
#15 = Methodref #6.#13 // java/security/cert/CRLException."<init>":(Ljava/lang/String;)V
#16 = Utf8 Code
{
java.lang.Throwable cause;
descriptor: Ljava/lang/Throwable;
flags:

org.bouncycastle.jce.provider.ExtCRLException(java.lang.String, java.lang.Throwable);
descriptor: (Ljava/lang/String;Ljava/lang/Throwable;)V
flags:
Code:
stack=2, locals=3, args_size=3
0: aload_0
1: aload_1
2: invokespecial #15 // Method java/security/cert/CRLException."<init>":(Ljava/lang/String;)V
5: aload_0
6: aload_2
7: putfield #14 // Field cause:Ljava/lang/Throwable;
10: return

public java.lang.Throwable getCause();
descriptor: ()Ljava/lang/Throwable;
flags: ACC_PUBLIC
Code:
stack=1, locals=1, args_size=1
0: aload_0
1: getfield #14 // Field cause:Ljava/lang/Throwable;
4: areturn
}

One processed with Jetifier:

Classfile /tmp/jetified/org/bouncycastle/jce/provider/ExtCRLException.class
Last modified Oct 14, 2019; size 362 bytes
MD5 checksum 76bc557706ee3480e53d88b21316913f
class org.bouncycastle.jce.provider.ExtCRLException extends java.security.cert.CRLException
minor version: 0
major version: 49
flags: ACC_SUPER
Constant pool:
#1 = Utf8 org/bouncycastle/jce/provider/ExtCRLException
#2 = Class #1 // org/bouncycastle/jce/provider/ExtCRLException
#3 = Utf8 java/security/cert/CRLException
#4 = Class #3 // java/security/cert/CRLException
#5 = Utf8 cause
#6 = Utf8 Ljava/lang/Throwable;
#7 = Utf8 <init>
#8 = Utf8 (Ljava/lang/String;Ljava/lang/Throwable;)V
#9 = Utf8 (Ljava/lang/String;)V
#10 = NameAndType #7:#9 // "<init>":(Ljava/lang/String;)V
#11 = Methodref #4.#10 // java/security/cert/CRLException."<init>":(Ljava/lang/String;)V
#12 = NameAndType #5:#6 // cause:Ljava/lang/Throwable;
#13 = Fieldref #2.#12 // org/bouncycastle/jce/provider/ExtCRLException.cause:Ljava/lang/Throwable;
#14 = Utf8 getCause
#15 = Utf8 ()Ljava/lang/Throwable;
#16 = Utf8 Code
{
java.lang.Throwable cause;
descriptor: Ljava/lang/Throwable;
flags:

org.bouncycastle.jce.provider.ExtCRLException(java.lang.String, java.lang.Throwable);
descriptor: (Ljava/lang/String;Ljava/lang/Throwable;)V
flags:
Code:
stack=2, locals=3, args_size=3
0: aload_0
1: aload_1
2: invokespecial #11 // Method java/security/cert/CRLException."<init>":(Ljava/lang/String;)V
5: aload_0
6: aload_2
7: putfield #13 // Field cause:Ljava/lang/Throwable;
10: return

public java.lang.Throwable getCause();
descriptor: ()Ljava/lang/Throwable;
flags: ACC_PUBLIC
Code:
stack=1, locals=1, args_size=1
0: aload_0
1: getfield #13 // Field cause:Ljava/lang/Throwable;
4: areturn
}

Changes in the constants pool cause changes in the code (indices change), which causes different MD5 checksums. This means that signatures will not match any more.

This started happening in 3.6.0-beta01 because we are using Jetifier with copyUnmodifiedLibsAlso=true flag (previously it was false).

hu...@google.com <hu...@google.com> #3Oct 25, 2019 03:09PM

@Chris You can work around this by updating gradle.properties with:

android.jetifier.blacklist=.*bcprov.*

This will avoid Jetifying the bouncycastle library.

vm...@google.com <vm...@google.com> #4May 24, 2023 12:35PM

Marked as fixed.

Thanks Ivan!

Issue 137929327

Description

Issue summary

Comments

cm...@google.com <cm...@google.com> #2Jul 22, 2019 01:35PM

hu...@google.com <hu...@google.com> #3Oct 25, 2019 03:09PM

vm...@google.com <vm...@google.com> #4May 24, 2023 12:35PM

Add comment

Issue metadata