Memory leak in 3.14.6

[Helena Åberg Östlund]

Hi Jooq team,

We want to report an issue with the latest release. We upgraded from 3.14.4 to 3.14.6 and then experienced a memory leak where the heap was slowly filling up with instances of org.jooq.impl.DefaultDataType instances.

We took a java heap dump of our service (a spring-boot service) and analysed it with Eclipse Memory Analyzer Tool. It showed that 70% of the memory was accumulated in a java.util.LinkedHashMap which contained these DefaultDataType objects. We didn't dig into this further, but the problem completely disappeared once we downgraded back to 3.14.4 so our conclusion is that the issue was with the new release.

Kind regards,

   Helena

[Lukas Eder]

Hi Helena,

Thank you very much for your report. That's bad news! We'll investigate this immediately and hope to be able to release 3.14.7 with a fix this week.

A few additional questions:

- Do you happen to know whether the regression happened in 3.14.5 or 3.14.6?

- Can you post that dump somewhere for us to analyse? (E.g. by private message to con...@datageekery.com)

- Did you do anything particular to produce this leak (e.g. use custom converters, bindings, etc.), or does it happen with any kind of jOOQ usage?

We'll try to reproduce this on our end and revert back ASAP.

Best Regards,

Lukas

--
You received this message because you are subscribed to the Google Groups "jOOQ User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jooq-user+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jooq-user/8ed0184c-f9a9-4ff0-a694-7908a3aabbc3n%40googlegroups.com.

[Lukas Eder]

This will be tracked as https://github.com/jOOQ/jOOQ/issues/11345

[Lukas Eder]

Tracking down changes made to the DataType related classes between 3.14.5 and 3.14.6, I think this is the most likely candidate for the regression:

https://github.com/jOOQ/jOOQ/issues/11086

Specifically, this commit could be creating increasing static Map sizes without ever cleaning things up:

https://github.com/jOOQ/jOOQ/commit/995cce0d6e60df9f7ba53962eec4eca5d1573d47

From what I can tell so far, this leak could happen when you create custom data types manually? Could that be something you're doing?

Thanks,

Lukas

[Helena Åberg Östlund]

Hello,

Don't know if it's in 3.14.5 as we never used that version, but I think what you found is the likely culprit.

Yes we do create custom data types.

I think it can be complicated to get approval for sharing the heap dump with you, but maybe you won't need it if you have identified the issue?

 /Helena

  I'm using Inbox When Ready to protect my focus.

You received this message because you are subscribed to a topic in the Google Groups "jOOQ User Group" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/jooq-user/4tM-YEuqISw/unsubscribe.
To unsubscribe from this group and all its topics, send an email to jooq-user+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jooq-user/2329f3ef-ff9e-4737-a220-f6ae6a1f506en%40googlegroups.com.

[Lukas Eder]

I probably don't need it, indeed. But a few sample pieces of code of how you create data types might be helpful. E.g. do you use DataType::asConvertedDataType or similar methods? Or do you subclass DefaultDataType or AbstractDataType, or something like that?

To view this discussion on the web visit https://groups.google.com/d/msgid/jooq-user/CACm_Rdg7JNMRqh0gTWSVf4xTngkRF4n6VmGNKKwgKz46YZ%3DSzQ%40mail.gmail.com.

[Helena Åberg Östlund]

We do things like this in our Maven pom.xml if that helps:

<database>
  <name>org.jooq.meta.postgres.PostgresDatabase</name>
  <includes>.*</includes>
  <inputSchema>public</inputSchema>
  <excludes>schema_version</excludes>
  <forcedTypes>
    <!--implicit conversion for DurationType-->
    <forcedType>
      <userType>com.spotify.sparta.model.api.DurationType</userType>
      <converter>com.spotify.sparta.model.data.DurationTypeConverter</converter>
      <includeExpression>(COMMITMENT_)?DURATION_TYPE</includeExpression>
      <includeTypes>DURATION_TYPE</includeTypes>
    </forcedType>

...

  /Helena

[Helena Åberg Östlund]

The custom types are configured in the pom.xml as in my previous post, so not sure how that works under the hood. Then we have code for converters for example like this:

package com.spotify.sparta.model.data;

import com.spotify.sparta.db.enums.JDurationType;
import com.spotify.sparta.model.api.DurationType;
import java.util.Locale;
import javax.annotation.Nullable;
import org.jooq.Converter;

/**
 * Converts between JDurationType and DurationType.
 * <p>
 * A JOOQ converter allows for two-way conversion between two Java data
 * types T and U. By convention, the T type corresponds to the
 * type in your database whereas the U type corresponds to your own user type.</p>
 */
public class DurationTypeConverter implements Converter<JDurationType, DurationType> {

  private static final long serialVersionUID = -7984438104416578889L;

  @Nullable
  @Override
  public DurationType from(JDurationType dbObject) {
    //this null check is required cause of jooq "null" object preparation before insert
    if (dbObject == null) {
      return null;
    }
    return DurationType.valueOf(dbObject.name().toUpperCase(Locale.ENGLISH));
  }

  @Override
  public JDurationType to(DurationType userObject) {
    return JDurationType.valueOf(userObject.name().toLowerCase(Locale.ENGLISH));
  }

  @Override
  public Class<JDurationType> fromType() {
    return JDurationType.class;
  }

  @Override
  public Class<DurationType> toType() {
    return DurationType.class;
  }
}

We have 15-20 of these user types.

Let me know if that is enough to go on or if you need something more.

 /Helena

To view this discussion on the web visit https://groups.google.com/d/msgid/jooq-user/CAB4ELO6OvvPNqEG0mneU2Ca4uuhZQDf0%2BGv%3D4BGZFyjkegY%3DvQ%40mail.gmail.com.

[Lukas Eder]

Thanks for the additional information. That's worse than I thought. This means it affects anyone's usage of converted data type, not just when using the DataType APIs manually, which is a bit more of a rare use-case.

I can reproduce the leak like this:

import org.jooq.impl.SQLDataType;

public class Leak {
    public static void main(String[] args) {
        for (;;) {
            SQLDataType.INTEGER.asConvertedDataType(Converters.identity(Integer.class));
        }
    }
}

It seems to reproduce relatively slowly, which is why this hasn't been reported earlier. But then again, 3.14.5 wasn't released to Maven Central because of some technical issues at sonatype, so I'm expecting this to impact more applications out there.

This patch against 3.14.6 seems to fix the leak, at least for my reproducer:

diff --git a/jOOQ/src/main/java/org/jooq/impl/DefaultDataType.java b/jOOQ/src/main/java/org/jooq/impl/DefaultDataType.java
index 729a9ec..e2ebebf 100644
--- a/jOOQ/src/main/java/org/jooq/impl/DefaultDataType.java
+++ b/jOOQ/src/main/java/org/jooq/impl/DefaultDataType.java
@@ -334,8 +334,8 @@
         if (TYPES_BY_TYPE[ordinal].get(type) == null)
             TYPES_BY_TYPE[ordinal].put(type, this);
 
-        if (TYPES_BY_SQL_DATATYPE[ordinal].get(this.sqlDataType) == null)
-            TYPES_BY_SQL_DATATYPE[ordinal].put(this.sqlDataType, this);
+        if (TYPES_BY_SQL_DATATYPE[ordinal].get(sqlDataType) == null)
+            TYPES_BY_SQL_DATATYPE[ordinal].put(sqlDataType, this);
 
         // Global data types
         if (dialect == null)

Do you have the means to test this patch on your end?

Best Regards,

Lukas

To view this discussion on the web visit https://groups.google.com/d/msgid/jooq-user/CACm_Rdgt73cnPLxKbSH0HaHc66ktFOsqxbfLdKv0GvTVFKr%2B0A%40mail.gmail.com.

[Helena Åberg Östlund]

Hi Lukas,

Glad that you found the issue and have a potential fix already! Unfortunately we cannot test the patch this week due to other priorities and we are happy to continue running on version 3.14.4 for now.

Thanks,

   Helena

To view this discussion on the web visit https://groups.google.com/d/msgid/jooq-user/CAB4ELO781O7EU8FEQ7rFimtjLSPBkfyHky-5-WgAN0%2BDR9BuLw%40mail.gmail.com.

[Lukas Eder]

Alright, no worries. Thanks again for your report! I'll release 3.14.7 today, after some more regression testing of the fix.

Best Regards,

Lukas

To view this discussion on the web visit https://groups.google.com/d/msgid/jooq-user/CACm_Rdgjk-xeh82%2Bvf7wC0myQs%2B%2BntbWqZPwvG%3DSr%3D7jD986TA%40mail.gmail.com.