JDBC PreparedStatement Memory Leak.

Can you send a context diff, diff -c? Thanks.

---------------------------------------------------------------------------

Leif Mortenson wrote:

I encountered a very large memory leak when using PreparedStatements in
a Java Application.

The memory leak only shows itself where all of the following are true:
1) The Threads which use the PreparedStatements have a long lifetime.
2) setDate() or setTimestamp is called on the PreparedStatements.
3) The PreparedStatements are being used enough to notice the leak.

In my application, my JVM was running out of memory around 330MB after
about 1 hour of
continuous opertion. I was able to find the problem by using the Java
Profiler. It showed
that around 100,000 instances of SimpleDateFormat and their subobjects
were hanging around
in memory.

It turns out that this was being caused by the way PreparedStatement was
using
ThreadLocal instances. Each one was creating a SimpleDateFormat which
via the
ThreadLocal gets set into a HashMap in the current Thread. These never
get cleaned
up until the Thread itself is GCed.

It looks like this had been discovered last summer and there were some
posts about a
patch having been committed. But the problem is still there.

Here is the patch that I made to fix this.

Cheers,
Leif

RCS file:
/projects/cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/jdbc2/PreparedStatement.java,v
retrieving revision 1.24
diff -w -r1.24 PreparedStatement.java
42,45c42,45
< // We use ThreadLocal for SimpleDateFormat's because they are not that
< // thread safe, so each calling thread has its own object.
< private static ThreadLocal tl_df = new ThreadLocal(); // setDate()
SimpleDateFormat
< private static ThreadLocal tl_tsdf = new ThreadLocal(); //
setTimestamp() SimpleDateFormat
---

// Store a copy of SimpleDataFormats for each instance and synchronize

them as they

// are not very thread safe.
private static SimpleDateFormat df; // setDate() SimpleDateFormat
private static SimpleDateFormat tsdf; // setTimestamp() SimpleDateFormat

351c351
< SimpleDateFormat df = (SimpleDateFormat) tl_df.get();
---

SimpleDateFormat df = this.df;

354,355c354,356
< df = new SimpleDateFormat("''yyyy-MM-dd''");
< tl_df.set(df);
---

// It is possible that two threads could get in here, but it
// does not cause problems.
this.df = df = new SimpleDateFormat("''yyyy-MM-dd''");

357c358,365
< set(parameterIndex, df.format(x));
---

String formatX;
synchronized(df)
{
// SimpleDateFormats are not thread safe.
formatX = df.format(x);
}
set(parameterIndex, formatX);

407c415
< SimpleDateFormat df = (SimpleDateFormat) tl_tsdf.get();
---

SimpleDateFormat df = tsdf;

409a418,419

// It is possible that two threads could get in here, but it
// does not cause problems.

412c422
< tl_tsdf.set(df);
---

tsdf = df;

424a435,441

String formatX;
synchronized(df)
{
// SimpleDateFormats are not thread safe.
formatX = df.format(x);
}

429c446
<
sbuf.append("'").append(df.format(x)).append('.').append(decimal).append("+00'");
---

sbuf.append("'").append(formatX).append('.').append(decimal).append("+00'");

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/users-lounge/docs/faq.html

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Leif,

In looking at this patch, I don't understand the bug that it is trying
to fix. I looked over how the ThreadLocal objects are being used in the
code and don't see how they could cause a memory leak. Can you explain
where the bug exists in the current logic? Or alternatively can you
provide a test case that demonstrates the problem so that I can look
into it some more here.

Also when submitting patches, please send them in diff -c format.

thanks,
--Barry

Bruce Momjian wrote:

Leif, can you respond to this?

---------------------------------------------------------------------------

Barry Lind wrote:

Leif,

In looking at this patch, I don't understand the bug that it is trying
to fix. I looked over how the ThreadLocal objects are being used in the
code and don't see how they could cause a memory leak. Can you explain
where the bug exists in the current logic? Or alternatively can you
provide a test case that demonstrates the problem so that I can look
into it some more here.

Also when submitting patches, please send them in diff -c format.

thanks,
--Barry

Bruce Momjian wrote:

Can you send a context diff, diff -c? Thanks.

---------------------------------------------------------------------------

Leif Mortenson wrote:

I encountered a very large memory leak when using PreparedStatements in
a Java Application.

The memory leak only shows itself where all of the following are true:
1) The Threads which use the PreparedStatements have a long lifetime.
2) setDate() or setTimestamp is called on the PreparedStatements.
3) The PreparedStatements are being used enough to notice the leak.

In my application, my JVM was running out of memory around 330MB after
about 1 hour of
continuous opertion. I was able to find the problem by using the Java
Profiler. It showed
that around 100,000 instances of SimpleDateFormat and their subobjects
were hanging around
in memory.

It turns out that this was being caused by the way PreparedStatement was
using
ThreadLocal instances. Each one was creating a SimpleDateFormat which
via the
ThreadLocal gets set into a HashMap in the current Thread. These never
get cleaned
up until the Thread itself is GCed.

It looks like this had been discovered last summer and there were some
posts about a
patch having been committed. But the problem is still there.

Here is the patch that I made to fix this.

Cheers,
Leif

RCS file:
/projects/cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/jdbc2/PreparedStatement.java,v
retrieving revision 1.24
diff -w -r1.24 PreparedStatement.java
42,45c42,45
< // We use ThreadLocal for SimpleDateFormat's because they are not that
< // thread safe, so each calling thread has its own object.
< private static ThreadLocal tl_df = new ThreadLocal(); // setDate()
SimpleDateFormat
< private static ThreadLocal tl_tsdf = new ThreadLocal(); //
setTimestamp() SimpleDateFormat
---

// Store a copy of SimpleDataFormats for each instance and synchronize

them as they

// are not very thread safe.
private static SimpleDateFormat df; // setDate() SimpleDateFormat
private static SimpleDateFormat tsdf; // setTimestamp() SimpleDateFormat

351c351
< SimpleDateFormat df = (SimpleDateFormat) tl_df.get();
---

SimpleDateFormat df = this.df;

354,355c354,356
< df = new SimpleDateFormat("''yyyy-MM-dd''");
< tl_df.set(df);
---

// It is possible that two threads could get in here, but it
// does not cause problems.
this.df = df = new SimpleDateFormat("''yyyy-MM-dd''");

357c358,365
< set(parameterIndex, df.format(x));
---

String formatX;
synchronized(df)
{
// SimpleDateFormats are not thread safe.
formatX = df.format(x);
}
set(parameterIndex, formatX);

407c415
< SimpleDateFormat df = (SimpleDateFormat) tl_tsdf.get();
---

SimpleDateFormat df = tsdf;

409a418,419

// It is possible that two threads could get in here, but it
// does not cause problems.

412c422
< tl_tsdf.set(df);
---

tsdf = df;

424a435,441

String formatX;
synchronized(df)
{
// SimpleDateFormats are not thread safe.
formatX = df.format(x);
}

429c446
<
sbuf.append("'").append(df.format(x)).append('.').append(decimal).append("+00'");
---

sbuf.append("'").append(formatX).append('.').append(decimal).append("+00'");

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/users-lounge/docs/faq.html

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/users-lounge/docs/faq.html

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026