SCM

[#1010965] NpgsqlConnection may throws on dispose

View Trackers | Bugs | Download .csv | Monitor

Date:
2010-12-21 18:20
Priority:
3
State:
Open
Submitted by:
Jonathan Oliver (joliver)
Assigned to:
Nobody (None)
Npgsql Version:
2.0.10
Category:
Group:
Resolution:
None
Summary:
NpgsqlConnection may throws on dispose

Detailed description
When a command is executed against a database which contains an explicit transaction, e.g.:

BEGIN TRANSACTION;

/* some work */

COMMIT TRANSACTION;

And when this command has some work which fails for one reason or another, e.g. a duplicate value violation or some other constraint violation, the COMMIT clause is never executed, as expected.

Also as expected, the connection is left in an unknown state and must be properly disposed. The issue arises because the code invoked through Dispose on the connection must perform some cleanup with the server. Specifically NpgsqlConnector.cs is invoked, and on line 381 the following code is called:

internal void ReleaseResources()
{
if (_connection_state != ConnectionState.Closed)
{
ReleasePlansPortals();
ReleaseRegisteredListen();
}
}

internal void ReleaseRegisteredListen()
{
Query(new NpgsqlCommand("unlisten *", this)); // throws
}

The disposal code must "unlisten", but this unlisten is still done within the context of the failed transaction. Any calls to the database in this state cause the database to return the following message:

current transaction is aborted, commands ignored until end of transaction block.

Of course "rollback" will never be called because it's implicit and by truly disposing and shutting down the connection we shouldn't have to either.

A few possible solutions include:
1. Wrapping the "unlisten" command in a try/catch
2. Prefixing the unlisten * command text with the following: ROLLBACK; unlisten *

While issuing a rollback inside of a command make look a little weird, its completely harmless in the case where a transaction has been successfully committed and it allows "unlisten" to execute properly.

A possible third alternative may be have the connector watch and understand the various exceptions and messages coming from the database and when it detects an aborted transaction, the connector could explicitly issue a rollback and then perform the actual clean/unlisten code.

By handling this situation, it will keep the connection from throwing on Dispose, which is the expected and appropriate behavior.

Followup

Message
Date: 2011-01-01 21:23
Sender: Francisco Figueiredo jr.


I think a better solution, as Josh said, is to remove the connection from the pool and close it explicitly in case of any error. This way we wouldn't need to deal with any error handling which in this case would be solved by a rollback but in other cases something else could be needed. I know we would hurt performance a little bit by closing the connection but it will be a very little overhead as other connections are available in the pool.

What do you think?
Date: 2010-12-22 18:11
Sender: Josh Cooley

I suspect the safest thing to do in this case is to remove the connector from the pool since the connection is involved in a transaction that is not managed by Npgsql. Any other recovery seems inherently unsafe since we would be trying to interpret the user's intent. I'll have to see what can be done to clean up after an exception (option 1).

Attached Files:

Attachments:
log.txt

Changes:

Field Old Value Date By
File Added583: log.txt2010-12-21 18:20joliver
Powered By FusionForge