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

Check commands[i] is not null for avoid NRE on Dispose command #22921

Closed
Tracked by #22949
eklimanov opened this issue Oct 8, 2020 · 6 comments · Fixed by #32581
Closed
Tracked by #22949

Check commands[i] is not null for avoid NRE on Dispose command #22921

eklimanov opened this issue Oct 8, 2020 · 6 comments · Fixed by #32581
Assignees
Labels
area-adonet-sqlite closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported punted-for-6.0 type-bug

Comments

@eklimanov
Copy link

eklimanov commented Oct 8, 2020

@bricelam FYI

Sometimes in inner collection _commands can be null value
image

internal void RemoveCommand(SqliteCommand command)
        {
            for (var i = _commands.Count - 1; i >= 0; i--)
            {
               //Could you check _commands[i] on null value here and delete this broken value?
                if (_commands[i].TryGetTarget(out var item)
                    && item == command)
                {
                    _commands.RemoveAt(i);
                }
            }
        }
@eklimanov eklimanov changed the title Check command is not null for avoid NPE on Dispose command Check commands[i] is not null for avoid NPE on Dispose command Oct 8, 2020
@bricelam
Copy link
Contributor

bricelam commented Oct 8, 2020

I expect all this code will change as part of #14044, but feel free to submit a PR in the meantime.

@lilith
Copy link

lilith commented Nov 29, 2020

Is there any workaround to this? I'm hitting this even in development.

@eklimanov
Copy link
Author

Also we face with next Exception. We can't any Commit or Rollback transaction. Could you help me with any workaround?

Message: OnException Exception Message: Object reference not set to an instance of an object.
System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.Data.Sqlite.SqliteConnection.RemoveCommand(SqliteCommand) + 0x3e
   at Microsoft.Data.Sqlite.SqliteCommand.Dispose(Boolean) + 0x21
   at System.ComponentModel.Component.Dispose() + 0xc
   at Microsoft.Data.Sqlite.SqliteConnectionExtensions.ExecuteNonQuery(SqliteConnection, String, SqliteParameter[]) + 0x22b
   at Microsoft.Data.Sqlite.SqliteTransaction.RollbackInternal() + 0x5c
   at Microsoft.Data.Sqlite.SqliteTransaction.Rollback()

@bricelam
Copy link
Contributor

bricelam commented Dec 5, 2020

Are you using the same connection on multiple threads? ADO.NET objects are generally not thread safe.

@ajcvickers
Copy link
Member

Just saw this on a local run of the EF tests:

 Microsoft.EntityFrameworkCore.Query.AdHocQuerySplittingQuerySqliteTest.Can_query_with_nav_collection_in_projection_with_split_query_in_parallel_sync
System.AggregateException : One or more errors occurred. (Object reference not set to an instance of an object.) (Object reference not set to an instance of an object.)\r\n---- System.NullReferenceException : Object reference not set to an instance of an object.\r\n---- System.NullReferenceException : Object reference not set to an instance of an object.
----- Inner Stack Trace #1 (System.NullReferenceException) -----
   at Microsoft.Data.Sqlite.SqliteConnection.RemoveCommand(SqliteCommand command) in C:\local\code\efcore\src\Microsoft.Data.Sqlite.Core\SqliteConnection.cs:line 451
   at Microsoft.Data.Sqlite.SqliteCommand.Dispose(Boolean disposing) in C:\local\code\efcore\src\Microsoft.Data.Sqlite.Core\SqliteCommand.cs:line 225
   at System.ComponentModel.Component.Dispose()
   at Microsoft.EntityFrameworkCore.Storage.RelationalDataReader.Dispose() in C:\local\code\efcore\src\EFCore.Relational\Storage\RelationalDataReader.cs:line 182
   at Microsoft.EntityFrameworkCore.Query.Internal.SplitQueryingEnumerable`1.Enumerator.Dispose() in C:\local\code\efcore\src\EFCore.Relational\Query\Internal\SplitQueryingEnumerable.cs:line 258
   at System.Linq.Enumerable.TryGetSingle[TSource](IEnumerable`1 source, Boolean& found)
   at lambda_method408828(Closure, QueryContext)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query) in C:\local\code\efcore\src\EFCore\Query\Internal\QueryCompiler.cs:line 68
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.Execute[TResult](Expression expression) in C:\local\code\efcore\src\EFCore\Query\Internal\EntityQueryProvider.cs:line 64
   at Microsoft.EntityFrameworkCore.Query.AdHocQuerySplittingQueryTestBase.<>c__DisplayClass15_0.<Can_query_with_nav_collection_in_projection_with_split_query_in_parallel_sync>g__Query|2(Guid parentId, Guid collectionId) in C:\local\code\efcore\test\EFCore.Relational.Specification.Tests\Query\AdHocQuerySplittingQueryTestBase.cs:line 216
   at Microsoft.EntityFrameworkCore.Query.AdHocQuerySplittingQueryTestBase.<>c__DisplayClass15_0.<Can_query_with_nav_collection_in_projection_with_split_query_in_parallel_sync>b__0() in C:\local\code\efcore\test\EFCore.Relational.Specification.Tests\Query\AdHocQuerySplittingQueryTestBase.cs:line 206
   at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot, Thread threadPoolThread)
--- End of stack trace from previous location ---
   at Microsoft.EntityFrameworkCore.Query.AdHocQuerySplittingQueryTestBase.Can_query_with_nav_collection_in_projection_with_split_query_in_parallel_sync() in C:\local\code\efcore\test\EFCore.Relational.Specification.Tests\Query\AdHocQuerySplittingQueryTestBase.cs:line 208
--- End of stack trace from previous location ---
----- Inner Stack Trace #2 (System.NullReferenceException) -----
   at Microsoft.Data.Sqlite.SqliteConnection.Close() in C:\local\code\efcore\src\Microsoft.Data.Sqlite.Core\SqliteConnection.cs:line 344
   at Microsoft.Data.Sqlite.SqliteConnection.Dispose(Boolean disposing) in C:\local\code\efcore\src\Microsoft.Data.Sqlite.Core\SqliteConnection.cs:line 415
   at System.ComponentModel.Component.Dispose()
   at Microsoft.EntityFrameworkCore.TestUtilities.RelationalTestStore.Dispose() in C:\local\code\efcore\test\EFCore.Relational.Specification.Tests\TestUtilities\RelationalTestStore.cs:line 52
   at Microsoft.EntityFrameworkCore.TestUtilities.TestStore.DisposeAsync() in C:\local\code\efcore\test\EFCore.Specification.Tests\TestUtilities\TestStore.cs:line 97
   at Microsoft.EntityFrameworkCore.NonSharedModelTestBase.DisposeAsync() in C:\local\code\efcore\test\EFCore.Specification.Tests\NonSharedModelTestBase.cs:line 148

@ajcvickers ajcvickers self-assigned this Dec 10, 2023
@ajcvickers
Copy link
Member

Note that this is because the test is re-using the same connection on multiple threads concurrently.

ajcvickers added a commit that referenced this issue Dec 10, 2023
Fixes #22921

Note that the reports here seem to be caused by incorrect use by the same connection concurrently. This is invalid. However, made the code a bit more robust here anyway.
@ajcvickers ajcvickers modified the milestones: Backlog, 9.0.0 Dec 10, 2023
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Dec 10, 2023
ajcvickers added a commit that referenced this issue Dec 11, 2023
Fixes #22921

Note that the reports here seem to be caused by incorrect use by the same connection concurrently. This is invalid. However, made the code a bit more robust here anyway.
@roji roji changed the title Check commands[i] is not null for avoid NPE on Dispose command Check commands[i] is not null for avoid NRE on Dispose command Dec 11, 2023
@ajcvickers ajcvickers modified the milestones: 9.0.0, 9.0.0-preview1 Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-adonet-sqlite closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported punted-for-6.0 type-bug
Projects
None yet
5 participants