On 12/28/2011 7:49 AM, Bergquist, Brett wrote:
>
> I created https://issues.apache.org/jira/browse/DERBY-5560 and am
> seeing this in production.
>
What an exciting holiday season you are having. I hope you didn't have
plans.
I haven't looked closely at this and probably won't have time to as I
have to peel away to complete some other tasks, but as an off the wall
remark, would it work to synchronize on "this" instead of
physicalConnection?
> Basically what is happening is that the LogicalConnection.close() is
> being called which attempts to recycle the physical connection by
> calling ClientPooledConnection.recycleConnection(). At the same time
> ClientPooledConnection.close() is being called which attempts to call
> LogicalConnection.nullPhysicalConnection(). The first thread holds
> a lock on LogicalConnection and needs the lock on
> ClientPooledConnection and the second thread holds a lock on
> ClientPolledConnection and needs a lock on LogicalConnection and a
> deadlock occurs.
>
> This is occurring because of the configuration of the connection pool
> in use (ClienXADataSource) and the pool is configured to close all
> connections on any error. The stack trace of the deadlock as a
> transaction being committed by the first thread and the connection
> pool closing all threads on a detected error in the second thread.
>
> I don't see immediately how to synchronize in an orderly way to
> eliminate the deadlock. The first thread has a handle on the
> LogicalConnection which references the physical
> ClientPooledConnection. The second thread has a handle on the
> physical ClientPooledConnection which references the LogicalConnection.
>
> One thought is to change the LogicalConnection.close to be something like:
>
> public void close() throws SQLException {
>
> synchronized(phsyicalConnection) {
>
> _close();
>
> }
>
> }
>
> synchronized public void _close() throws SQLException {
>
> try
>
> {
>
> // we also need to loop thru all the logicalStatements and
> close them
>
> if (physicalConnection_ == null) {
>
> return;
>
> }
>
> if (physicalConnection_.agent_.loggingEnabled()) {
>
> physicalConnection_.agent_.logWriter_.traceEntry(this,
> "close");
>
> }
>
> if (physicalConnection_.isClosed()) // connection is
> closed or has become stale
>
> {
>
> pooledConnection_.informListeners(new SqlException(null,
>
> new ClientMessageId(
>
> SQLState.PHYSICAL_CONNECTION_ALREADY_CLOSED)));
>
> } else {
>
> physicalConnection_.checkForTransactionInProgress();
>
> physicalConnection_.closeForReuse(
>
> pooledConnection_.isStatementPoolingEnabled());
>
> if (!physicalConnection_.isGlobalPending_()) {
>
> pooledConnection_.recycleConnection();
>
> }
>
> }
>
> physicalConnection_ = null;
>
> pooledConnection_.nullLogicalConnection();
>
> }
>
> catch ( SqlException se )
>
> {
>
> throw se.getSQLException();
>
> }
>
> }
>
> but this has a problem if "physicalConnection" is already null.
>
> Any guidance will be greatly appreciated as I need to patch Derby
> 10.8.2.2 to work around this issue in any case.
>
> Brett
>
|