Обсуждение: Unexpected NullPointerException in "processDeadParsedQueries()" - suggested fix
Unexpected NullPointerException in "processDeadParsedQueries()" - suggested fix
От
Morten Andersen
Дата:
Hi, In a multithreaded application I just got an unexpected NullPointerException from the depth's of the JDBC-drivers cleanup routine of parsed queries. The driver I am using is the most recent JDBC3 driver, version 8.2-504. During a batch-execution of a trivial PreparedStatement that generally works fine I got the NullPointerException below: Exception in thread "Thread-5" java.lang.NullPointerException at org.postgresql.core.Utils.encodeUTF8(Utils.java:54) at org.postgresql.core.v3.QueryExecutorImpl.sendCloseStatement(QueryExecutorImpl.java:982) at org.postgresql.core.v3.QueryExecutorImpl.processDeadParsedQueries(QueryExecutorImpl.java:1104) at org.postgresql.core.v3.QueryExecutorImpl.sendQueryPreamble(QueryExecutorImpl.java:364) at org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:322) at org.postgresql.jdbc2.AbstractJdbc2Statement.executeBatch(AbstractJdbc2Statement.java:2592) [SNIP - rest of calling stacktrace - was outside the driver] I have been browsing the CVS-repository and I think the code that generates this is the processDeadParsedQueries() below: private void processDeadParsedQueries() throws IOException { PhantomReference deadQuery; while ((deadQuery = (PhantomReference)parsedQueryCleanupQueue.poll()) != null) { String statementName = (String)parsedQueryMap.remove(deadQuery); <--- RETURNS null? sendCloseStatement(statementName); <--- TRIGGERS NullPointerException deadQuery.clear(); } } I inserted the comments with "<---". The reaon why it is possible for the parsedQueryMap to return a null statementName for the deadQuery key is quite a puzzle to me. Mostly I can only think of two reasons: 1) The statementName for the deadQuery has already been removed. Given the code for registerParsedQuery() and processDeadParsedQueries() I can't see how this should be possible, as the java.lang.ref.ReferenceQueue is thread-safe (it doesn't say so in the API documentation, but that is what you would expect, and a quick glance at the JDK source also indicates that this is the case). 2) The use of HashMap for the parsedQueryMap could be a problem in a multithreaded application, since it is not thread-safe? So a wild guess is that the thread executing the processDeadParsedQueries() does not "see" the value that was added to the hashmap by another thread in the registerParsedQuery. Maybe some of you guys can see other ways this situation could arise? Otherwise I would suggest a combination of two solutions: a) change parsedQueryMap to a synchronizedMap (either by the Collections.synchronizedMap or by using a Hashtable - or for JDK1.5+ the more "highspeed" java.util.concurrent.ConcurrentHashMap). and b) do an explicit check that the returned statementName is not null, i.e. change the body of the while-loop in processDeadParsedQueries to something along the lines: String statementName = (String)parsedQueryMap.remove(deadQuery); if (statementName != null) { sendCloseStatement(statementName); } deadQuery.clear(); This should not be necessary (as far as I can tell?), since the ReferenceQueue should never contain the deadQuery before it has been added to the parsedQueryMap in registerParsedQuery() due to the contract for PhantomReference? But one should always be carefull with this kind of Thread-reasoning :-) Please feel to come with any thoughts on this fix, and if you need more information on the exact code that executed the PreparedStatement, please also mail me. Best Regards Morten Andersen, Instadia A/S, Denmark
On Wed, 20 Dec 2006, Morten Andersen wrote: > In a multithreaded application I just got an unexpected NullPointerException > from the depth's of the JDBC-drivers cleanup routine of parsed queries. > Is this something you can reproduce somewhat regularly or something that just happened once? > During a batch-execution of a trivial PreparedStatement that generally works > fine I got the NullPointerException below: > > Exception in thread "Thread-5" java.lang.NullPointerException > at org.postgresql.core.Utils.encodeUTF8(Utils.java:54) > at > org.postgresql.core.v3.QueryExecutorImpl.sendCloseStatement(QueryExecutorImpl.java:982) > at > org.postgresql.core.v3.QueryExecutorImpl.processDeadParsedQueries(QueryExecutorImpl.java:1104) > at > > I have been browsing the CVS-repository and I think the code that generates > this is the processDeadParsedQueries() below: > > private void processDeadParsedQueries() throws IOException { > PhantomReference deadQuery; > while ((deadQuery = (PhantomReference)parsedQueryCleanupQueue.poll()) != > null) > { > String statementName = (String)parsedQueryMap.remove(deadQuery); <--- > RETURNS null? > sendCloseStatement(statementName); <--- TRIGGERS NullPointerException > deadQuery.clear(); > } > } > > I inserted the comments with "<---". > > The reaon why it is possible for the parsedQueryMap to return a null > statementName for the deadQuery key is quite a puzzle to me. Mostly I can > only think of two reasons: > > 1) The statementName for the deadQuery has already been removed. Given the > code for registerParsedQuery() and processDeadParsedQueries() I can't see how > this should be possible, as the java.lang.ref.ReferenceQueue is thread-safe > (it doesn't say so in the API documentation, but that is what you would > expect, and a quick glance at the JDK source also indicates that this is the > case). > > 2) The use of HashMap for the parsedQueryMap could be a problem in a > multithreaded application, since it is not thread-safe? So a wild guess is > that the thread executing the processDeadParsedQueries() does not "see" the > value that was added to the hashmap by another thread in the > registerParsedQuery. > Neither of these look particularly likely to me. All entry points to QueryExecutorImpl that access these items are synchronized so I don't think the thread safety of the individual items are an issue. Is your application multi-threaded and using a Connection per thread or does it use multiple threads per connection? If using Connection per thread it's even harder to believe this is a thread safety issue because you'll have a QueryExecutorImpl per Connection. Kris Jurka
Hi Kris, Kris Jurka wrote: > >> In a multithreaded application I just got an unexpected >> NullPointerException from the depth's of the JDBC-drivers cleanup >> routine of parsed queries. >> > > Is this something you can reproduce somewhat regularly or something that > just happened once? Unfortunately it is not reproduceable :-) - I just saw it once, in a long running application with heavy load on the driver. [SNIP] > > Neither of these look particularly likely to me. All entry points to > QueryExecutorImpl that access these items are synchronized so I don't > think the thread safety of the individual items are an issue. After re-inspection, I think you are right. I definitely had a too limited "scope" when looking for the cause, since I mostly/only looked at registerOpenPortal() and processDeadPortals(). > > Is your application multi-threaded and using a Connection per thread or > does it use multiple threads per connection? If using Connection per > thread it's even harder to believe this is a thread safety issue because > you'll have a QueryExecutorImpl per Connection. Multi-threaded, with only one thread using one connection at a time, but using an Apache Commons Pool, so over time the same connection will be used from several threads. But given the issues with the synchronization on all entry points, as you pointed out, I think you are right, in that neither of my two suggestions to what the cause of the original stacktrace could be, are correct. So since I don't have any other good ideas to what it could be, and since I only saw it once (and nobody else reported a similar issue), we should probably just ignore the error as not-reproducable. But thanks for taking your time to answer, and for looking into this issue. Best Regards Morten Andersen, Instadia A/S, Denmark