Thread: JDBC PreparedStatement Memory Leak.
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'");
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: > 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 >> > >
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