| Re: [eclipselink-dev] Connection leak with Apache DBCP - changing connection close logic? |
Hi Sabine, It should be ok to still call close even if the connection is closed. Maybe remove the isConnected check entirely (as it can be expensive) and call the close in a finally (in case the prerelease or clear fails). Any errors are already trapped an ignored, so things should be ok. You will need to test it, but I can’t foresee it having issues. Please log a bug. From: Heider, Sabine [mailto:sabine.heider@xxxxxxx] Hi all, I’d like to get your help and opinions on a quite annoying connection leak problem that most of our applications suffer from. We experience connection leaks when EclipseLink is used with an external Apache Commons DBCP pool and the physical connection drops off due to a n!
etwork
problem, database shutdown or similar. We have found that the problem happens at the interface between EclipseLink and the DBCP Pool (the pooled connection, to be precise) and is caused by the kind of unclear semantics of the Connection.isClosed method. The issue occurs in the DatasourceAccessor.closeConnection() method: /** * Close the accessor's connection. * This is used only for external connection pooling * when it is intended for the connection to be reconnected in the future. */ public void closeConnection() { try { if (this.datasourceConnection != null) { if (isDatasourceConnected()) { // <----- here’s the issue if(currentSession != null) { currentSession.preReleaseConnection(this); } if(customizer != null && customizer.isActive()) { customizer.clear(); } closeDatasourceConnection(); } this.datasourceConnection = null; } } catch (DatabaseException exception) { // Ignore this.datasourceConnection = null; } finally { currentSession = null; } } The method first checks Connection.isClosed and only closes the connection if that method returns false. The Connection.isClosed implementation of Apache’s PooledConnection, howe!
ver,
returns true if either the connection handle is closed or the underlying physical connection is closed. In our case, a temporary network problem or database restart causes the physical connection to be closed: The isClosed method returns true, EclipseLink never closes the connection and therefore the connection never gets returned to the pool :-( I checked the Javadoc of the Connection.isClosed method. It says: “Retrieves whether this Connection object has been closed. A connection is closed if the method close has been called on it or if certain fatal errors have occurred. This method is guaranteed to return true only when it is called after the method Connection.close has been called. This method generally cannot be called to determine whether a connection to a database is valid or
invalid. A typical client can determine that a connection is invalid by catching any exceptions that might be thrown when an operation is attempted. “ Pretty fuzzy :-( I understand from the doc that the only defined state is if you have called Connection.close before (method returns true). If you haven’t, there’s basically no guarantee for anything – I’m not quite sure whether it’s a good idea to base a connection closing decision on that. Our connection leak could be solved by changing the closeConnection() logic such that closeDatasourceConnection() would be called in any case. What’s your opinion on that? Is that feasible at all? Would that cause other issues? The plain Connection.close method can be called multiple times, so (theoretically) there shouldn’t be!
an
issue, but how about the rest of the connection handling logic and/or the other accessors? Should I open a bug? What do you think? Thanks for your help and best regards, Sabine Sabine Heider SAP AG Pflichtangaben/Mandatory Disclosure Statements: http://www.sap.com/company/legal/impressum.epx |