Re: JDBC PreparedStatement Memory Leak. - Mailing list pgsql-patches

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