QueryFactory IDisposable?
generik0 opened this issue · 7 comments
Memory/ dbconnection leak when using QueryFactory. Why is QueryFactory disposable when passing the bConnection, I
understand if the passing the connection string, but when passing the connection, the owner of the connection should be responsible for passing it, no?
On the document i see an example:
https://sqlkata.com/docs/execution/fetching-records
var db = new QueryFactory(connection, new SqlServerCompiler());
Are we sure the current implementation is correct? Or should the documentation be corrected?
I made a PR to add using statements to the doco.
Don't know what to do about the query factory
HI @ahmad-moussawi , are you sure SqlKata.Execution.QueryExtensions isn't giving a memory leak, you are newing the QueryFactory on each call, never disposing.. But QueryFactory is IDisposable, I think you should be disposing the CreateQueryFactory otherwise you will the conection leaks lie we did...
@generik0 talking in this manner is not acceptable,
If you have a comment or feedback, you have one of two options:
- Ask politely and provide valuable feedback, or
- Help and contribute with a solution
other than that I am obliged to ignore your requests
Sorry, I don't agree I was being rude, I have not added not adjectives. But a little aggressive, I can agree upon.
Hence I have edited my original message...
This memory leak was quite a serious thing for us, it took us weeks to find, as we looked everywhere else first. That isn't your problem, it's ours, but attributed to the frustrations.
Btw, I did help contribute, did you see the PR on your doco repo @ahmad-moussawi ;-)
Also I went though many of the questions and saw if I could answer. You might note remember me helping with some mssql code back in the 0.5.x days.
I also started on adding integrations test to prove the memory leak. It ran out of time. I can imagine you have a similar issue...
Any thoughts about this potential memory leak?
I was thinking 2 QueryFactories, one for the connection string, that owns the connections, one that doesn't..
Please note @ahmad-moussawi
The question about the memory leak above is on the extensions is very relevant.
What is disposing the QueryFactory? It is IDisposable. Will the GC dispose it?
It didn't for us, when we needed it, but here we kept it in a field until the scope was over.
I know the example with the DI regression will dispose it.
@generik0 do you have a reproducible scenario, so it help point out the issue easily?
Yes. I will add some integration tests you your sln (fork). What project would you like me to call it?
Sql.Kata.IntegrationTests?
I Will admit i am wrong. Even though the QueryFacrory is IDisoposable, the extensions are not giving a leak.
The leak i got was in relation the to the DI not disposing the Factory properly, the way We where using it.
I do still think have the QueryFactory as disposable, an it storing the connection is wrong, it should be stateless. But that is my opinion...