Issue Details (XML | Word | Printable)

Key: DNET-678
Type: Bug Bug
Status: Closed Closed
Resolution: Won't Fix
Priority: Major Major
Assignee: Jiri Cincura
Reporter: Hans Torm
Votes: 1
Watchers: 1
Operations

If you were logged in you would be able to see more operations.
.NET Data provider

FbConnectionPoolManager does not remove disconnected FbConnectionInternal instances

Created: 05/Apr/16 09:03 AM   Updated: 21/Feb/17 07:42 AM
Component/s: ADO.NET Provider
Affects Version/s: 4.10.0.0
Fix Version/s: None

Environment: The scenario have been tested versus FB 2.5 in classic mode
Issue Links:
Relate
 


 Description  « Hide
If a FbConnection's FbConnectionInternal gets disconnected from the server in the underlying System.Net.Sockets.NetworkStream, this is not detected by the FbConnectionInternal instance. Thus the FbConnectionInternal instance will keep on being released to the FbConnectionPoolManager upon Close and reused for later connections, creating errors.

The problem is compounded by the fact that even if you detect the Error, you cannot simply Dispose the FbConnection as this will still release the FbConnectionInternal instance to the Pool. Only workaround is to flush the Pool, but to do that you have to break the encapsulation provided by the ADO.NET abstractions and link directly to the FbDriver to get access to the Pool management features on FbConnection, thus preventing DB agnostic code. This is not desirable.

Alternatively not disposing a failed connection will work as the FbConnectionInternal instance will never be GC'd due to being in the _busy List, but will of course introduce a subtle memory leak. This is not desirable.


 All   Comments   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Jiri Fartak added a comment - 15/Nov/16 11:30 AM
Hi Jiri,

I didn't study the whole architecture of .Net provider, however what about of the idea of adding connection status property like "IsOpen" to the FbConnectionInternal whose getter would propagate this call to underlying IDatabase's socket? I think that indirect testing of underlying Socket's status via polling would do the needed behavior.

The FbConnectionPoolManager is the resource owner/factory and thus it should provide valid resource when someone asks for it. Further, it can destroy the broken connections on these occasions:

1) When the FBConnectionInternal is returned to the pool (after FBConnection.Dispose() method call)- if the test (IsOpen call) would fail, then FbConnectionINternal is destroyed immediately and not included into the _available sub-pool.

2) When the FbConnectionPoolManager is asked for acquring a resource - existing or new connection - then after any available connection is popped up from the _available stack, then if IsOpen call would fail, then such connection is destroyed immediately and this would repeat until any opened connection is found. If so, then it is returned. This process would also naturally clean up the _available sub-pool. If no such connection is found (_available.Count reaches zero), then new one is created. By other words acquiring method (in pseudo code) would look like:

public FbConnectionInternal GetConnection(FbConnection owner) would do the thing:
{
lock (_syncRoot)
{
CheckDisposedImpl();

FbConnectionInternal connection = null;
while(_available.Any())
{
//this loop naturally cleans up broken connections

connection = _available.Pop().Connection;
if (!connection.IsOpen) //test the connection state (polling)
{
Destroy(connection); --- this would immediately destroys(GC) the connection
}
}
if (connection == null)
connection = CreateNewConnectionIfPossibleImpl(_connectionString, owner); //the pool is empty (either all were all destroyed since they were broken or nothing was available), let's try to create new one
connection.SetOwningConnection(owner);
_busy.Add(connection);
return connection;
}
}


3) GC (when its time comes) can pro-actively polls the FBConnectionInternal.IsOpen flag and if false is returned then such broken connections will be destroyed and collected.

The fact, that broken connections can live for some time in the _available sub-pool, is not problem. Yes, they will stay there either until GC thread will collect them or when acquiring them for next use.

Jiri Cincura added a comment - 21/Feb/17 07:42 AM
The problem is that there's no way to know whether the socket is connected. The Connected reflects the state of the connection as of the most recent operation. So there would have to be dummy operation (making everything slower). And even that. When the connection is returned from pool because the IsOpen was true, there's no guarantee it's still opened at the point of returning it to developer. That's why the DNET-668 was introduced.