You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@jmeter.apache.org by bu...@apache.org on 2012/10/12 09:56:30 UTC

[Bug 53995] New: AbstractJDBCTestElement shares PreparedStatement between multi-threads

https://issues.apache.org/bugzilla/show_bug.cgi?id=53995

          Priority: P2
            Bug ID: 53995
          Keywords: PatchAvailable
          Assignee: issues@jmeter.apache.org
           Summary: AbstractJDBCTestElement shares PreparedStatement
                    between multi-threads
          Severity: critical
    Classification: Unclassified
                OS: Linux
          Reporter: starfish.hu@gmail.com
          Hardware: PC
            Status: NEW
           Version: 2.7
         Component: Main
           Product: JMeter

Created attachment 29468
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=29468&action=edit
The patch for JMeter 2.7-rc3, fix the shared PreparedStatement bug.

The implementation of AbstractJDBCTestElement uses a map to cache the
PreparedStatement for each Connection, and shares that cache between
multi-threads, which is the cause of the bug.

According to the section 13.1.4 of JDBC 4.0 specification
(http://jcp.org/aboutJava/communityprocess/final/jsr221/index.html), 

> 13.1.4 Closing Statement Objects
> An application calls the method Statement.close to indicate that it 
> has finished processing a statement. All Statement objects will be closed
> when the con- nection that created them is closed. However, it is good coding
> practice for applications to close statements as soon as they have finished 
> processing them. This allows any external resources that the statement is 
> using to be released immediately.
> Closing a Statement object will close and invalidate any instances of 
> ResultSet produced by that Statement object. The resources held by the
> ResultSet object may not be released until garbage collection runs again, 
> so it is a good practice to explicitly close ResultSet objects when they 
> are no longer needed.
> Once a Statement has been closed, any attempt to access any of its methods 
> with the exception of the isClosed or close methods will result in a
> SQLException being thrown.
> These comments about closing Statement objects apply to PreparedStatement 
> and CallableStatement objects as well.

It's clear that closing a PreparedStatement object will automatically close all
the ResultSet objects opened by it. Therefore, if a PreparedStatement is shared
between thread A and thread B, and the thread A is closing the
PreparedStatement object while thread B is still in use of a ResultSet object
opened by that PreparedStatement object, the ResultSet object will be closed
indirectly by thread A without notifying thread B. This situation could happen,
since the cache of PreparedStatement has a capacity limit and it will
automatically close an old cached PreparedStatement object while adding a new
PreparedStatement to the cache reaching the capacity limit.

It's also easy to reproduce the bug: create a JDBC testing plan using a
prepread statement, with 300 threads and 2000 loops; then run the test, the bug
*may* occur. If the bug does not occur, try to increase the number of threads
and the number of loops. The log of the JDBC driver shows that it is caused by
using a closed ResultSet object.

The attachment is the patch for Apache JMeter v2.7-rc3.

The patch does the following works:

1. change the cache of PreparedStatement in AbstractJDBCTestElement to a thread
local cache;
2. add some logging statements in order to help locating the bug;
3. fix the location of "jcharts" component in the "build.properties"
configuration file, since the "www.mvnsearch.org" was moved.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 53995] AbstractJDBCTestElement shares PreparedStatement between multi-threads

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53995

Philippe Mouawad <p....@ubik-ingenierie.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEEDINFO                    |NEW

--- Comment #2 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
Ok I saw issue, the issue is that one thread A can reach limit of perConnCache
generating a closeAllStatements() which can impact thread B using this cache.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 53995] AbstractJDBCTestElement shares PreparedStatement between multi-threads

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53995

--- Comment #3 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
Created attachment 29476
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=29476&action=edit
Test Plan showing issue

To provoke bug, set:
jdbcsampler.cachesize=1

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 53995] AbstractJDBCTestElement shares PreparedStatement between multi-threads

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53995

Philippe Mouawad <p....@ubik-ingenierie.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #5 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
Date: Sat Oct 27 14:50:28 2012
New Revision: 1402802

URL: http://svn.apache.org/viewvc?rev=1402802&view=rev
Log:
Bug 53995 - AbstractJDBCTestElement shares PreparedStatement between
multi-threads
Bugzilla Id: 53995

Modified:
   
jmeter/trunk/src/protocol/jdbc/org/apache/jmeter/protocol/jdbc/AbstractJDBCTestElement.java
    jmeter/trunk/xdocs/changes.xml

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 53995] AbstractJDBCTestElement shares PreparedStatement between multi-threads

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53995

Philippe Mouawad <p....@ubik-ingenierie.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |53978

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 53995] AbstractJDBCTestElement shares PreparedStatement between multi-threads

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53995

Philippe Mouawad <p....@ubik-ingenierie.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Hardware|PC                          |All
                 OS|Linux                       |All

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 53995] AbstractJDBCTestElement shares PreparedStatement between multi-threads

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53995

Philippe Mouawad <p....@ubik-ingenierie.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |NEEDINFO
                 CC|                            |p.mouawad@ubik-ingenierie.c
                   |                            |om

--- Comment #1 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
I am not sure your analysis is correct.
There are 2 modes for JDBC Connection Configuration.

If max pool is set to 0, there will be one pool for each thread, connections
will never be shared, so this one is OK

Now, if max pool is set, it can happen at one time that 2 threads will use the
same connection, but in my understanding, it will never happen at the same time
as pool will return an idle connection (so different from the one in use) or it
will wait if no one is available.

So the case you describe cannot happen.

It is true that the pool shares PreparedStatement but they are always
associated to the same connection. So I don't think there is an issue as
connection will never be shared by two threads at the same time, but I may be
wrong.

Could you submit a Test Plan showing the issue and the exact error ? or one of
the 2 issues ?

Thank you.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 53995] AbstractJDBCTestElement shares PreparedStatement between multi-threads

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53995

Philippe Mouawad <p....@ubik-ingenierie.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|53978                       |

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 53995] AbstractJDBCTestElement shares PreparedStatement between multi-threads

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53995

--- Comment #4 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
Thank you for the reporting, the analysis and the patch.
The proposed patch has the impact of creating one Cache per Thread and
connection which increases memory usage during test.

I implemented another fix which removes the MRU cache as it is the source of
issue, since the put of a connection or PreparedStatement Map could result into
the putter thread provoking a cleanup (and close) of PreparedStatement of other
thread.

This should not impact very much memory as size of first map is the size of
number of threads.
Size of second map is the size of different SQL queries of the test.

-- 
You are receiving this mail because:
You are the assignee for the bug.