Deadlock when loading DataType classes

[tstack]

We're seeing a deadlock when two different threads load different DataType classes at the same time. It looks like loading one DataType class causes the others to be loaded as well during class initialization. But, this can cause a deadlock when multiple threads are trying to load these DataType classes because they each hold a lock on the class and end up waiting for the others to finish.

The following code reproduces the deadlock for me:

import java.io.*;
import java.net.*;

class jooqlock {
    public static void main(String[] args) throws Exception {
        for (int i = 0; i < 1; i++) {
            doit();
        }
    }

    private static void doit() throws Exception {
        final ClassLoader loader = new URLClassLoader(new URL[] { new File("jooq-3.4.4.jar").toURI().toURL() });

        Runnable oload = new Runnable() {
                public void run() {
                    try {
                        Class.forName("org.jooq.util.sqlite.SQLiteDataType", true, loader);
                    } catch (Throwable th) {
                        th.printStackTrace();
                    }
                }
            };
        Runnable sload = new Runnable() {
                public void run() {
                    try {
                        Class.forName("org.jooq.util.postgres.PostgresDataType", true, loader);
                    } catch (Throwable th) {
                        th.printStackTrace();
                    }
                }
            };

        Thread th1 = new Thread(oload);
        Thread th2 = new Thread(sload);
        th1.start();
        th2.start();
        th1.join();
        th2.join();
    }
}

Here is a stack dump from after the deadlock happens:

Full thread dump Java HotSpot(TM) 64-Bit Server VM (24.65-b04 mixed mode):

"Thread-1" prio=5 tid=0x00007fd119841800 nid=0x5303 in Object.wait() [0x000000011b619000]
   java.lang.Thread.State: RUNNABLE
    at org.jooq.util.postgres.PostgresDataType.<clinit>(PostgresDataType.java:76)
    at java.lang.Class.forName0(Native Method)
    at java.lang.Class.forName(Class.java:270)
    at jooqlock$2.run(jooqlock.java:27)
    at java.lang.Thread.run(Thread.java:745)

"Thread-0" prio=5 tid=0x00007fd119841000 nid=0x5103 in Object.wait() [0x000000011b515000]
   java.lang.Thread.State: RUNNABLE
    at java.lang.Class.forName0(Native Method)
    at java.lang.Class.forName(Class.java:190)
    at org.jooq.impl.SQLDataType.<clinit>(SQLDataType.java:341)
    at java.lang.Class.forName0(Native Method)
    at java.lang.Class.forName(Class.java:190)
    at org.jooq.impl.DefaultDataType.<clinit>(DefaultDataType.java:219)
    at org.jooq.util.sqlite.SQLiteDataType.<clinit>(SQLiteDataType.java:71)
    at java.lang.Class.forName0(Native Method)
    at java.lang.Class.forName(Class.java:270)
    at jooqlock$1.run(jooqlock.java:18)
    at java.lang.Thread.run(Thread.java:745)

[lukaseder]

Thank you very much for letting us know. Yes, this can indeed happen, specifically because of the static block in org.jooq.impl.SQLDataType, which we implemented to prevent that sort of issue:

    static {
        // Load all dialect-specific data types
        // TODO [#650] Make this more reliable using a data type registry

        try {
            Class.forName(CUBRIDDataType.class.getName());
            Class.forName(DerbyDataType.class.getName());
            Class.forName(FirebirdDataType.class.getName());
            Class.forName(H2DataType.class.getName());
            Class.forName(HSQLDBDataType.class.getName());
            Class.forName(MariaDBDataType.class.getName());
            Class.forName(MySQLDataType.class.getName());
            Class.forName(PostgresDataType.class.getName());
            Class.forName(SQLiteDataType.class.getName());
        } catch (Exception ignore) {}
    }

Before we look into fixing this, could you tell me why this might happen in your specific use-case? Do you have generated classes that refer to the dialect-specific data types early on?

[tstack]

We have some helper classes that are initialized early on and refer to the DataType classes, we're not using generated classes. We also use multiple database types at the same time in different threads in the jvm. Our fix is to load jooq separately for each thread since they already had their own class loaders.

I imagine our situation is not very common, but we wanted to let you know.

[lukaseder]

I imagine our situation is not very common, but we wanted to let you know.

Yes, it's not very common, but I guess we were taking bets when we implemented the above, and Murphy's law (as always) struck. In fact, we knew that it would strike (see #2322), but we couldn't think of a scenario to produce that issue...

Thanks for letting us know about this. We'll think of a fix that'll make initialisation more robust.

[lukaseder]

Related #2322

[lukaseder]

I've found two possible solutions:

1. Use reflection to check whether the class is already loaded. This is more of a quick fix. Example here: http://stackoverflow.com/a/482909/521799
2. Create a new class that initialises all dialects, and dialect-specific data types, and let PostgresDataType.* simply be references to the other class's data types.

The latter would be much more robust in the sense of #2322. But I think it won't make it into 3.5.0 anymore. It'll need some more thinking.

[gundeman]

We have also run into this issue. We worked around it by running this little code block on the applications main thread before starting up other threads:

  try {
        Class.forName("org.jooq.impl.DefaultDataType");
    } catch (ClassNotFoundException e) {
        e.printStackTrace();
    }

[lukaseder]

@gundeman: Thank you very much for documenting this

[victorbr]

We happened to bump into this issue and while digging a bit into it I was wondering why the initialization lines in SQLDataType look like

Class.forName(CUBRIDDataType.class.getName());

Unless I'm mistaken I believe it's an overkill - as soon as the code mentions CUBRIDDataType.class, it causes the class to be loaded. Looking it up again by name does nothing besides actually creating this deadlock.
I have tried replacing these lines by

Class<?> temp = CUBRIDDataType.class;

etc. and the deadlock no longer occurs.
@lukaseder do you think it could be a viable solution?

[lukaseder]

Thanks, @victorbr. Interesting thoughts. Since these deadlock occurred, there certainly had been quite a few changes to jOOQ's internals. I'll investigate your suggestion, soon.

[victorbr]

Just to clarify - this is easily reproducible on 3.10.0 with a test as simple as

    public static void main(String[] args) throws Exception {
        Thread th1 = new Thread(() -> SQLDataType.VARCHAR(6));
        Thread th2 = new Thread(() -> PostgresDataType.VARCHAR.length(6));
        th1.start();
        th2.start();
        th1.join();
        th2.join();
    }

(the original test in this thread, simplified)

[lukaseder]

Thanks for the additional heads-up

[lukaseder]

Yeah, well about that suggestion:

Class<?> temp = CUBRIDDataType.class;

Try running this at the end of your main method:

System.out.println(DSL.using(SQLDialect.MYSQL).render(cast(field("x"), SQLDataType.VARCHAR(6))));

Unfortunately, referencing the class doesn't necessarily load it. And if you did anything like referencing a member of the class, such as CUBRIDDataType.VARCHAR, you're back to the deadlock, as that would still have the same effect.

It's really a tricky problem to solve. I suspect it can only be solved like this: #5713, i.e. by getting rid of the internal static caches in DefaultDataType...

[victorbr]

Try running this at the end of your main method:

Not sure I got this. With my version (referencing .class) it prints

cast(x as varchar(6))

With 3.10.0 it prints

cast(x as char(6))

[lukaseder]

Yeah, well the point is, cast(x as varchar(6)) is illegal in MySQL. You can only cast to char. The dialects and their datatypes haven't been initialised correctly when you apply your fix.

[victorbr]

Aha! I see! Though I'm kinda puzzled by the current state - there's a clear circular dependency between SQLDataType and e.g. MySQLDataType static sections. I wonder how class loader manages to resolve it in the "normal" case...
I would love to help, just need to understand the intent of the current implementation! Appreciate your help!

[lukaseder]

Yes, there's a circular dependency, which isn't a problem if only one thread loads the classes:

• SQLDataType is loaded first, and with it, all standard data types are initialised
• When all SQLDataType content is initialised (but the class isn't completely loaded yet), then the dependencies (dialect-specific data types) are loaded. They can now refer to parent content, even if the class isn't completely initialised

This is a hack that persisted from early days on, and it works quite well in most cases. Every now and then, someone runs into this and the workaround is always to pre-initialise the data types.

This is not a nice situation of course, and it does need fixing.

[lukaseder]

The main reason for this deadlock to happen is because end user code references <dialect>DataType instances and triggers class loading of dialect specific data types before loading the SQLDataType class. This doesn't work and is very hard to fix. We're going to deprecate <dialect>DataType classes and remove them from public API in the future: #7375.

This seems to be the most reliable way to prevent these deadlocks without implementing the rather difficult change #5713.

[lukaseder]

We'll consider this issue fixed through the deprecation of the offending "internal" API: #7375. If it re-appears in other contexts, we can reopen this issue.

[etiennestuder]

@lukaseder I don't understand what the replacement for the deprecated class is, i.e. we are using org.jooq.util.postgres.PostgresDataType#TEXT, what should I replace this with in our code?

In general, it is really helpful when deprecating something to also mention what the new replacement is.

[lukaseder]

@etiennestuder

I don't understand what the replacement for the deprecated class is

You should reference SQLDataType only

i.e. we are using org.jooq.util.postgres.PostgresDataType#TEXT, what should I replace this with in our code?

The corresponding SQLDataType type is SQLDataType.CLOB

In general, it is really helpful when deprecating something to also mention what the new replacement is.

Yes, I'm well aware of this "best practice", thanks for mentioning this. I'll fix the Javadoc of the <dialect>DataType classes, this was merely an oversight.

[etiennestuder]

Thanks for the insights, @lukaseder.

[raulvc]

sorry for bumping this dead post, any idea what's an equivalent for postgres DOUBLE PRECISION (float8) ?

[lukaseder]

@raulvc May I ask you to create a new issue with more details? https://github.com/jOOQ/jOOQ/issues/new/choose

[JoseLion]

@lukaseder I'm using JOOQ only for migrations (with Flyway), so I would like to have the exact PostgreSQL data types when I create the tables (instead of the SQL equivalents). Just as an example, if I add a column with type PostgresDataType.TEXT it will be created as text on the database, but with SQLDataType.VARCHAR the type will be varchar in the database instead 😞

If we want to keep PostgreSQL specific data types, what should we do? 🤔

[lukaseder]

If we want to keep PostgreSQL specific data types, what should we do? 🤔

In general, please use a data type binding and attach that to the code generator's output:
https://www.jooq.org/doc/latest/manual/code-generation/custom-data-type-bindings

However, I think that it's currently not possible (without resorting to string manipulation) to override the data type literal when generating DDL statements.

[JoseLion]

I see 🤔 as I'm using it for migrations only, my best options is to go back to plain SQL files. However, thank you for the quick response on closed issue @lukaseder! 🙂