Bug #75849 NPE in abortInternal() method on line 1358 of ConnectionImpl
Submitted: 10 Feb 2015 17:58 Modified: 4 Sep 2015 21:46
Reporter: Torsten Krah Email Updates:
Status: Closed Impact on me:
None 
Category:Connector / J Severity:S2 (Serious)
Version:5.1.34 OS:Linux
Assigned to: Alexander Soklakov CPU Architecture:Any

[10 Feb 2015 17:58] Torsten Krah
Description:
In case of killing my application via signal or Strg+C i am forcefully closing/aborting my currently in use mysql connections with the abort(Executor x) method of JDBC 4.1.

Got about 16 connections in use in the pool and sometimes (not everytime) when i am aborting them, i've get a NPE from:

this.io.releaseResources();

Looking at the code there maybe a race condition from realClose(...) which does also set this.io to null when finished.
So if any Thread is calling close which results in realClose on the connection and at the same time the abortInternal method is called, it may result in a NPE here.
It may also happen on Line 1354 where 

this.io.forceClose();

is called, but the NPE would be hidden from the Throwable catch around.

Finishing the IO object should be guarded by some synchronization, to prevent this NPE to happen.

How to repeat:
Just use many concurrent Threads which are calling close on connections and abort them at the same time.

Suggested fix:
Fix would be to use something like:

 synchronized (getConnectionMutex()) {
  ...
 }

"close()" itself is guarded too with this mutex, abortInternal uses the io object too, imho it should be guarded there too.
[13 Feb 2015 10:27] Alexander Soklakov
Hi Torsten,

Thank you for the report.

This problem doesn't have a good solution, current implementation allows the race conditions but the synchronization here could cause deadlocks in the same situation.

Actually the race condition isn't critical at this stage of connection, having this.io == null means that we already passed all these final steps, physical connection is closed and we may just dismiss the connection object. The wrong behavior is that NPE is thrown to the application level, it should be masked, i.e. this.io.releaseResources() should be placed along with this.io.forceClose() into try-catch.
[4 Sep 2015 21:46] Daniel So
Added the following entry to the Connector/J 5.1.37 changelog:

"When a connection is forcefully closed with abortInternal() in the ConnectionImpl class, a null point exception sometimes resulted. This is now avoided by putting the associated this.io.releaseResources() call inside a try block, so that the exception, unavoidable due to a race condition, can be properly caught and ignored."