Thread: JDBC PreparedStatement Memory Leak.

JDBC PreparedStatement Memory Leak.

From
Leif Mortenson
Date:
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'");




Re: JDBC PreparedStatement Memory Leak.

From
Bruce Momjian
Date:
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

Re: JDBC PreparedStatement Memory Leak.

From
Barry Lind
Date:
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
>>
>
>



Re: JDBC PreparedStatement Memory Leak.

From
Bruce Momjian
Date:
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