Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ado net provider causes firebird server to leak handles until it is unable to process any requests any longer [DNET852] #784

Closed
firebird-automations opened this issue Sep 20, 2018 · 27 comments

Comments

@firebird-automations
Copy link

Submitted by: Dominik Psenner (dpsenner)

Is related to DNET274
Is duplicated by DNET830

Attachments:
PseudoApplication.zip

After an update of the firebird http://ado.net driver from 5.12.0 to 5.12.1 or 6.3 we observe that the firebird server 2.5.x leaks memory that is related to a connection and/or the statements that a client runs against the firebird database using the http://ado.net provider > 5.12.0.

We observe both an increase in the private memory set (4GB) but also the nonpaged pool increases significantly (5000K) in the firebird server process until the firebird server process is no longer able to handle any statements. At this point the server returns IscError 335544761 "too many open handles to database" and the SQLSTATE is "HY000". This causes the offending client to crash.

As soon as the connection closes, the firebird server is able to recover. It does then free the allocated memory and the nonpaged pool decreases back to normal (<160K).

This may relates to changes implemented with in release 5.12.1. Before http://ado.net provider 5.12.1, the connection pool to the firebird server did not work properly and connections were closed too early. Connections used to be always short-living. 5.12.1 fixed that and from then on connections are properly pooled. This causes connections to become long-living as long as there is activity. If connections are not closed, the server end will not end up the resources it has allocated and sooner or later show the symptom observed.

We cannot disable the connection pool because of performance reasons. We cannot revert back to 5.12.0 because the connection pool would not work either and we have no further option. Therefore I set the priority of this issue to blocker.

We do also have a minimal sample application that reproduces the issue with a very simplistic database containing only one table with three columns.

@firebird-automations
Copy link
Author

Modified by: Dominik Psenner (dpsenner)

summary: Firebird server leaking handles using .net provider > 5.12.0.0 => Ado net provider causes firebird server to leak handles until it is unable to process any requests any longer

@firebird-automations
Copy link
Author

Commented by: Dominik Psenner (dpsenner)

I have attached the reproducing sample application to this issue.

@firebird-automations
Copy link
Author

Modified by: Dominik Psenner (dpsenner)

Attachment: PseudoApplication.zip [ 13300 ]

@firebird-automations
Copy link
Author

Commented by: Dominik Psenner (dpsenner)

Added the firebird versions and flavors affected.

@firebird-automations
Copy link
Author

Modified by: Dominik Psenner (dpsenner)

environment: Windows 7, Windows Server 2016 => Windows 7, Windows Server 2016, Firebird 2.5.3, Firebird 2.5.4, Firebird 2.5.8, Firebird superserver, Firebird superclassic, Firebird classic

@firebird-automations
Copy link
Author

Modified by: @cincuranet

status: Open [ 1 ] => In Progress [ 3 ]

@firebird-automations
Copy link
Author

Commented by: @cincuranet

OK, the cause of the problem is that EF6 is not explicitly disposing commands. I'm investigating how is the finalizer catching up and how the `ReleasePreparedCommands` fits into the play, because it should take care of it eventually.

@firebird-automations
Copy link
Author

Modified by: @cincuranet

Link: This issue is related to DNET274 [ DNET274 ]

@firebird-automations
Copy link
Author

Modified by: @cincuranet

Link: This issue is duplicated by DNET830 [ DNET830 ]

@firebird-automations
Copy link
Author

Commented by: Dominik Psenner (dpsenner)

Please don't implement dispose strategies that rely on the garbage collector. The GC is indeterministic and therefore not the tool to release unmanaged resources like command handles on the firebird server.

I just investigated how the npgsql http://ado.net provider releases database commands and it seems there is no special handling involved in that implementation. See the EF6 adapter implementation at [1] and [2]. As far as I can tell, everything relies on the invocation of IDisposable.Dispose(). NpgsqlCommand does even request the GC to suppress finalizing the instance because it implements the disposable pattern and frees up the resources by itself, see [3]. NpgsqlConnection does no special handling either and implements dispose and calls GC.SuppressFinalize() too. On dispose it frees up the NpgsqlConnector instance that tracks the current interaction with the npgsql server (see [4]).

[1] https://github.com/npgsql/EntityFramework6.Npgsql/blob/dev/src/EntityFramework6.Npgsql/NpgsqlServices.cs
[2] https://github.com/npgsql/EntityFramework6.Npgsql/blob/dev/src/EntityFramework6.Npgsql/NpgsqlConnectionFactory.cs
[3] https://github.com/npgsql/npgsql/blob/dev/src/Npgsql/NpgsqlCommand.cs
[4] https://github.com/npgsql/npgsql/blob/dev/src/Npgsql/NpgsqlConnection.cs

@firebird-automations
Copy link
Author

Commented by: @cincuranet

This is issue that has root cause in EF6 - i.e. dotnet/ef6#580 or more precisely dotnet/ef6#639.

@firebird-automations
Copy link
Author

Modified by: @cincuranet

status: In Progress [ 3 ] => Closed [ 6 ]

resolution: Won't Fix [ 2 ]

@firebird-automations
Copy link
Author

Commented by: Dominik Psenner (dpsenner)

Interestingly, when I update the sample application to read as follows, the dispose method on the fbconnection is invoked but the statements are not freed on the server. With this, the owner of the connection is the calling application and the calling application ensures disposal of the connection. Note that the connection string still has pooling enabled. Shouldn't all resources of a connection be freed on disposal, even if it is a pooled one?

            using \(var dbConnection = new FbConnection\(connectionString\)\)
            using \(var dbContext = new PseudoDbContext\(dbConnection, false\)\)
            using \(var transaction = dbContext\.BeginReadWriteTransaction\(\)\)
            \{
                Console\.WriteLine\("Opened connection \+ transaction"\);
                int itemsToRead = 1;
                if \(isBurst\)
                \{
                    itemsToRead = 100;
                \}

                for \(int i = 0; i < itemsToRead; i\+\+\)
                \{
                    ReadItem\(random, dbContext\);
                \}

                Console\.WriteLine\("Disposed connection \+ transaction"\);
            \}

@firebird-automations
Copy link
Author

Commented by: @cincuranet

Kind of. When the connection is disposed, alive resources are disposed. But because connection tracks commands via weak references, some commands might have been finalized already. I vaguely remember probably 10 years ago the change to weak references because of some other memory "leak" by something (don't remember what it was anymore), but this was "leaking" on client side.

Thinking about it now, before it was done because in the code there was lot of wrong finalizers, which I've recently cleaned up. So it might actually make sense to use regular references.

I wish I remember clearly what exactly the issue was, to see whether the change is safe. Also I'm not sure how is this going to play out when one FbCommand would be used with multiple connections. I'll have to try that out.

@firebird-automations
Copy link
Author

Commented by: @cincuranet

Created a ticket for that - DNET854. Shouldn't take too long to try it out.

@firebird-automations
Copy link
Author

Commented by: Dominik Psenner (dpsenner)

> connection tracks commands via weak references

I do hope that on the client, handles that reference resources on the server are not weak references. If that's the case the client is allowed to remove instances that reference server side resources to save memory. This in turn surfaces as very interesting symptoms like the server leaking handles and memory. :-)

> Also I'm not sure how is this going to play out when one FbCommand would be used with multiple connections.

In all rdbms db commands are tightly related to server resources. DbConnection offers the CreateCommand() method for a reason. Developers shall fix the code if they happen to find such a usage.

@firebird-automations
Copy link
Author

Commented by: @cincuranet

> I do hope that on the client, handles that reference resources on the server are not weak references. If that's the case the client is allowed to remove instances that reference server side resources to save memory. This in turn surfaces as very interesting symptoms like the server leaking handles and memory. :-)

Well, as long as you keep the reference somewhere in rooted variable, it doesn't matter because the finalizer will not be able to collect that command. And if you've forgot to dispose it and lost the reference, well, it's your fault anyway.

@firebird-automations
Copy link
Author

Commented by: Dominik Psenner (dpsenner)

Arthur Vickers closed dotnet/ef6#639 with won't fix. EF providers shall implement a workaround because EF6 is not going to implement any dispose on instances that implement the disposable pattern. Ideally such a workaround is limited to the EF6 integration only but it will be hard since EF6 does not inform the provider when it does no longer need a command.

@firebird-automations
Copy link
Author

Commented by: @cincuranet

That's what DNET854 is about, if it proves to be doable. At the moment you can clear the pool regularly to really disconnect and then server will free the statements.

@firebird-automations
Copy link
Author

Commented by: Dominik Psenner (dpsenner)

Is there a way to use a custom connection pool?

@firebird-automations
Copy link
Author

Commented by: @cincuranet

What is a custom connection pool?

@firebird-automations
Copy link
Author

Commented by: Dominik Psenner (dpsenner)

The current fb connection pool implementation keeps connections open with a configurable min lifetime. I thought of force disposing connections after a configurable maximum lifetime. For instance, connections could be kept alive for a minimum of 15 seconds as long as the connection handles commands but is closed after at most 5 minutes to recycle any leaked handles.

@firebird-automations
Copy link
Author

Commented by: @cincuranet

You can't do that directly, but you can use FbConnection.ClearPool after max lifetime was exceeded. Different pools you can get by using different values in connection string, for example `ClientLibrary` can be used as dummy value that's changing.

@firebird-automations
Copy link
Author

Commented by: Dominik Psenner (dpsenner)

After having a conversation in the team, we chose to stick with 5.12.0 without implementing any hacks or workarounds. The reasoning is that the software is going to behave like it did until now and we already learned how to handle that. This also means to us that we will have to re-evaluate our options in the near future. We do hope that until then the dotnet firebird provider implements a workaround such that it does no longer cause the firebird server to leak handles when used with EF6. We do further hope that the connection pool is improved such that it recycles connections that exceed a configurable maximum lifetime. Thanks for your work and your valuable insights. Cheers!

@firebird-automations
Copy link
Author

Commented by: @cincuranet

Maximum lifetime is surely not going to be implemented. It's not something that's "standard" in http://ADO.NET.

@firebird-automations
Copy link
Author

Commented by: Dominik Psenner (dpsenner)

The reasoning behind such a configuration is that, while not standard among http://ado.net providers, it is an effective workaround against all kind of resource leaks that are disposed when a connection is closed.

Would you accept a contributed pull request on github?

@firebird-automations
Copy link
Author

Commented by: @cincuranet

I would very likely reject that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants