PropertyNamingStrategy class initialization depends on its subclass, this can lead to class loading deadlock

[fangwentong]

If a class refer to its subclasses in its static initializers or in static fields, this references can cause JVM-level deadlocks in multithreaded environment, when one thread tries to load superclass and another thread tries to load subclass at the same time.

See: https://bugs.openjdk.java.net/browse/JDK-8037567

The following demo code can reproduce this deadlock

public class ClassInitDeadLock {

    public static void main(String[] args) throws InterruptedException {
        Thread threadA = new Thread(new Runnable() {
            @Override
            public void run() {
                // trigger supper class initialize
                PascalCaseStrategy strategy = PropertyNamingStrategy.PASCAL_CASE_TO_CAMEL_CASE;
            }
        }, "Thread-A");

        Thread threadB = new Thread(new Runnable() {
            @Override
            public void run() {
                // trigger sub class initialize
                PascalCaseStrategy strategy = new PascalCaseStrategy();
            }
        }, "Thread-B");

        threadA.start();
        Thread.sleep(100);
        threadB.start();
    }

    public static class PropertyNamingStrategy {
        static {
            try {
                // sleep 1s for deadlock reproduction
                Thread.sleep(1000);
            } catch (InterruptedException e) {
                // ignore
            }
        }

        public static final PascalCaseStrategy PASCAL_CASE_TO_CAMEL_CASE = new PascalCaseStrategy();
    }

    public static class PascalCaseStrategy extends PropertyNamingStrategy {
    }
}

When the demo program ClassInitDeadLock started, it could not exit automatically. Thread-A holding PropertyNamingStrategy class , and waiting PascalCaseStrategy class initialize, while Thread-B holding PascalCaseStrategy class, and waiting PropertyNamingStrategy class initialize, deadlock occurred!

jstack of these two thread:

"Thread-A" #11 prio=5 os_prio=31 tid=0x00007fa8c29d8000 nid=0xa803 in Object.wait() [0x000070000a4c4000]
   java.lang.Thread.State: RUNNABLE
	at ClassInitDeadLock$PropertyNamingStrategy.<clinit>(ClassInitDeadLock.java:35)
	at ClassInitDeadLock$1.run(ClassInitDeadLock.java:8)
	at java.lang.Thread.run(Thread.java:748)

"Thread-B" #12 prio=5 os_prio=31 tid=0x00007fa8c4a9b800 nid=0x5603 in Object.wait() [0x000070000a5c7000]
   java.lang.Thread.State: RUNNABLE
	at ClassInitDeadLock$2.run(ClassInitDeadLock.java:16)
	at java.lang.Thread.run(Thread.java:748)

In the actual environment, PropertyNamingStrategy and its subclass defined in jackson-databind initialize very fast, so It's hard to produce the scene that multithread load superclass and subclass at the same time.

I happened to meet this deadlock recently, It turn out to be JsonErrorUnmarshaller.java#L37 defined in aws-java-sdk-core, trying to initialize subclass PascalCaseStrategy, at the same time, another thread is initilizing superclass PropertyNamingStrategy

private static final ObjectMapper MAPPER = new ObjectMapper().configure(
        DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false).setPropertyNamingStrategy(
        new PascalCaseStrategy());

[cowtowncoder]

Interesting. Thank you for reporting this, sounds like a potentially severe issue.

Do you know version of Jackson problem was encountered with? (AWS jdk core used relatively old version last I checked, 2.6.7?, but that was a while ago).
I assume issue has been around for a while but just wanted to check.

[cowtowncoder]

Also... may be really really difficult to actually resolve, wrt backwards compatibility problems.

[fangwentong]

As a workarround, we can change the code of aws-java-sdk-core, new PascalCaseStrategy() => PropertyNamingStrategy.PASCAL_CASE_TO_CAMEL_CASE, this change can bypass the problem.

[cowtowncoder]

@fangwentong Yes. I think for databind itself, will probably need to introduce something like PropertyNamingStrategies to hold instances (which would have been good to do from beginning as a pattern). And then deprecate sub-type static instance.

[fangwentong]

OK, thanks, backwards compatibility is really important，I'll create a PR to aws-java-sdk-core.

[cowtowncoder]

Unfortunately I think the solution can only go in 2.12, as I need to add PropertyNamingStrategies as the place for static instances, deprecate existing entries; that is, making API changes that need to go in a new minor version.