NullReferenceException when transaction rollback

[Gankov]

Software versions MySqlConnector version: 2.2.6 Server type (MySQL, MariaDB, Aurora, etc.) and version: MariaDB (versions different 10.x) .NET version: 4.6.1 (Optional) ORM NuGet packages and versions: Nhibernate 5.3 and 5.4

Describe the bug Sometimes when transaction rollback we have NullReferenceException. How i can check before call rollback that i can get NullReferenceException? This code worked with MySQL.Data. Perhaps for MySQLConnector need check something else? it seems like connection still open.

Exception

System.Reflection.TargetInvocationException: Адресат вызова создал исключение. ---> System.NullReferenceException: Ссылка на объект не указывает на экземпляр объекта.
   в MySqlConnector.MySqlConnection.<CloseDatabaseAsync>d__140.MoveNext()
--- Конец трассировка стека из предыдущего расположения, где возникло исключение ---
   в System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   в System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   в MySqlConnector.MySqlConnection.<DoCloseAsync>d__139.MoveNext()
--- Конец трассировка стека из предыдущего расположения, где возникло исключение ---
   в System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   в MySqlConnector.MySqlConnection.<DoCloseAsync>d__139.MoveNext()
--- Конец трассировка стека из предыдущего расположения, где возникло исключение ---
   в System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   в System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   в MySqlConnector.MySqlConnection.<CloseAsync>d__138.MoveNext()
--- Конец трассировка стека из предыдущего расположения, где возникло исключение ---
   в System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   в System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   в MySqlConnector.MySqlConnection.Dispose(Boolean disposing)
   в System.ComponentModel.Component.Dispose()
   в NHibernate.Connection.DriverConnectionProvider.CloseConnection(DbConnection conn)
   в NHibernate.Impl.NonContextualConnectionAccess.CloseConnection(DbConnection connection)
   в NHibernate.AdoNet.ConnectionManager.AggressiveRelease()
   в NHibernate.AdoNet.ConnectionManager.AfterTransaction()
   в NHibernate.Transaction.AdoTransaction.AfterTransactionCompletion(Boolean successful)
   в NHibernate.Transaction.AdoTransaction.Rollback()
   в QS.DomainModel.UoW.UnitOfWorkBase.DisposeUoW()
   в QS.DomainModel.UoW.UnitOfWorkBase.Dispose()
   в QS.ViewModels.Dialog.EntityDialogViewModelBase`1.Dispose()
   в Autofac.Core.Disposer.Dispose(Boolean disposing)
   в Autofac.Util.Disposable.Dispose()
   в Autofac.Core.Lifetime.LifetimeScope.Dispose(Boolean disposing)
   в Autofac.Util.Disposable.Dispose()
   в QS.Navigation.AutofacViewModelsTdiPageFactory.<>c__DisplayClass4_0`1.<MakePage>b__0(Object sender, PageClosedEventArgs e)
   в QS.Navigation.PageBase.QS.Navigation.IPageInternal.OnClosed(CloseSource source)
   в QS.Navigation.NavigationManagerBase.ClosePage(IPage page, CloseSource source)
   в QS.Navigation.TdiNavigationManager.ClosePage(IPage page, CloseSource source)
   в QS.Navigation.TdiNavigationManager.TdiNotebook_TabClosed(Object sender, TabClosedEventArgs e)
   в QS.Tdi.Gtk.TdiNotebook.OnTabClosed(ITdiTab tab, CloseSource source)
   в QS.Tdi.Gtk.TdiNotebook.CloseTab(TdiTabInfo info, CloseSource source)
   в QS.Tdi.Gtk.TdiNotebook.ForceCloseTab(ITdiTab tab, CloseSource source)
   в QS.Tdi.Gtk.TdiNotebook.AskToCloseTab(ITdiTab tab, CloseSource source)
   в QS.Tdi.Gtk.TdiNotebook.OnCloseButtonClicked(Object sender, EventArgs e)
   --- Конец трассировки внутреннего стека исключений ---
   в System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor)
   в System.Reflection.RuntimeMethodInfo.UnsafeInvokeInternal(Object obj, Object[] parameters, Object[] arguments)
   в System.Delegate.DynamicInvokeImpl(Object[] args)
   в GLib.Signal.ClosureInvokedCB(Object o, ClosureInvokedArgs args)
   в GLib.SignalClosure.Invoke(ClosureInvokedArgs args)
   в GLib.SignalClosure.MarshalCallback(IntPtr raw_closure, IntPtr return_val, UInt32 n_param_vals, IntPtr param_values, IntPtr invocation_hint, IntPtr marshal_data)

Code sample

        protected virtual void DisposeUoW()
        {
            if(transaction != null) {
                if(!transaction.WasCommitted && !transaction.WasRolledBack
                && transaction.IsActive && session.Connection.State == System.Data.ConnectionState.Open)
                    transaction.Rollback();

                transaction.Dispose();
                transaction = null;
            }
            Session.Dispose();
            UowWatcher.UnregisterUow(this);
        }

[bgrainger]

If the call stack is accurate, then it seems like the exception is happening here: https://github.com/mysql-net/MySqlConnector/blob/2.2.6/src/MySqlConnector/MySqlConnection.cs#L1098-L1107

I'm not seeing where the exception could come from unless CurrentTransaction is not null but m_session is.

Is it possible that there is a background thread doing work on this MySqlConnection at the same time that OnCloseButtonClicked is clicked in the UI? Sharing connections between threads is not permitted: https://mysqlconnector.net/troubleshooting/connection-reuse/.

[Gankov]

It's potentially possible, the app is big enough, but I don't see where it would happen. The application basically doesn't use threads directly in the database related code. Unless in an obvious way.

I see in MySqlConnection.cs#L1098-L1107 and can't understand what throw NullReferenceException too.

I see that in MySqlConnector called async methods. Possibly we call Dispose multi times and parallel thread created in MySqlConnector. I will try fix multi times disposing.

[stilettk]

Even if the connection was used in unsupported scenarios, throwing NullReferenceException is not a good practice. In this case is can be easily fixed:

- if (CurrentTransaction is not null && m_session!.IsConnected)
+ if (CurrentTransaction is not null && m_session?.IsConnected is true)

[bgrainger]

That would only be an inappropriate change if m_session is expected to be null; otherwise it's communicating false semantics about the expected preconditions/invariants.

It first needs to be established that that's a valid state to be in (as opposed to misuse of the library, in which case any invariants might be invalidated and all bets are off).