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

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
>>
>
>