Issue22921 #23346
Issue22921 #23346
Conversation
Check if command[i] equals null and remove this from collection.
| @@ -418,8 +418,9 @@ internal void RemoveCommand(SqliteCommand command) | |||
| { | |||
| for (var i = _commands.Count - 1; i >= 0; i--) | |||
| { | |||
| if (_commands[i].TryGetTarget(out var item) | |||
| && item == command) | |||
| if (_commands[i] == null | |||
There was a problem hiding this comment.
No. I mean == null. If we check != null then the collection can't delete null values and this will cause memory leaks. Please look at my screenshot in linked Issues.
|
@eklimanov Can you please add tests that verify that this change fixes the problem? |
I don't know how to insert null values into this collection. In our project, such a situation arose when we make several requests to DB in parallel. We make synchronization with a lot of HTTP calls (up to 1500) and async-await code where we save all response data in our local DB. We use one transaction for fast saving up to 200 rows in one table. We use one transaction if two responses come together. And sometimes we face this bug. But this bug is floating. It comes after 5 maybe 10-minute of synchronization. After one zero value gets into this collection, all command Disposing and all transaction commands stop working for us. We wrapped all Dispose of command in try-catch. But this not good. Also, please read #20651 for similar behavior. |
|
@eklimanov At this point, I don't think we know what the correct fix is here. This will require debugging of the code to figure out the root cause, followed by or at the same time as adding new tests to cover the issue filed and any related scenarios that might be effected. We will of course do this work when we fix the bug. However, if you want to send a PR to fix it sooner, then that would be appreciated, but this is going to require that you figure out the root cause and correct fix and add appropriate tests to verify this. |
|
Closing due to lack of activity. |
Fix: #22921