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

NullReferenceException in SQLite.PreparedSqlLiteInsertCommand.ExecuteNonQuery(...) #761

Closed
COM8 opened this issue Sep 28, 2018 · 15 comments
Closed

Comments

@COM8
Copy link

COM8 commented Sep 28, 2018

Description:

I'm getting from time to time if I start my app a System.NullReferenceException: Object reference not set to an instance of an object.
I'm unable to reproduce it, but once it happens once it will occur about 2 to 3 times in a row. After that the app starts again as expected. It could be that this error is related to #530 but I'm not sure. I'm accessing the DB heavily multithreaded.

Details:

  • sqlite-net version: 1.5.231 occurs also on 1.4.118 and 1.3.3
  • Implementation
  • UWP target version: 16299
  • UWP min version: 10586

Stacktrace:

   at SQLite.PreparedSqlLiteInsertCommand.ExecuteNonQuery(Object[] source)
   at SQLite.SQLiteConnection.Insert(Object obj, String extra, Type objType)
   at Thread_Save_Components.Classes.SQLite.TSSQLiteConnection.InsertOrReplace(Object obj)
   at XMPP_API.Classes.Network.XML.DBManager.OmemoDeviceDBManager.setDevices(List`1 devices, String chatJid)
   at XMPP_API.Classes.Network.XML.DBManager.OmemoDeviceDBManager.setDevices(OmemoDevices devices, String chatJid, String accountId)
   at XMPP_API.Classes.Network.OmemoHelper.onOmemoDeviceListEventMessage(OmemoDeviceListEventMessage msg)
   at XMPP_API.Classes.Network.XMPPConnection2.<parseMessageAsync>d__53.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
   at XMPP_API.Classes.Network.XMPPConnection2.<TCPConnection_NewDataReceived>d__56.MoveNext()
@Kipotlov
Copy link

Kipotlov commented Oct 5, 2018

Hi,
I commented on issue #530. I add my comment here too.

It crash because the member variable "Connection" is null, on this line :
if (Connection.Trace)
Please note that it happends during multithread access.

Here is the call stack in your code :
SQLite-net.dll!SQLite.PreparedSqlLiteInsertCommand.ExecuteNonQuery(object[] source) Line 2989
SQLite-net.dll!SQLite.SQLiteConnection.Insert(object obj, string extra, System.Type objType) Line 1667
SQLite-net.dll!SQLite.SQLiteConnection.InsertOrReplace(object obj, System.Type objType) Line 1596

@Kipotlov
Copy link

Kipotlov commented Oct 5, 2018

Ok, after a look in the code, i think i found the problem.
In this method, if added is false, prepCmd is disposed then return...

PreparedSqlLiteInsertCommand GetInsertCommand (TableMapping map, string extra)
{
	PreparedSqlLiteInsertCommand prepCmd;
	var key = Tuple.Create (map.MappedType.FullName, extra);

	lock (_insertCommandMap) {
		_insertCommandMap.TryGetValue (key, out prepCmd);
	}

	if (prepCmd == null) {
		prepCmd = CreateInsertCommand (map, extra);
		var added = false;
		lock (_insertCommandMap) {
			if (!_insertCommandMap.ContainsKey (key)) {
				_insertCommandMap.Add (key, prepCmd);
				added = true;
			}
		}
		if (!added) {
			prepCmd.Dispose ();
		}
	}

	return prepCmd;
}

COM8 added a commit to UWPX/UWPX-Client that referenced this issue Oct 28, 2018
COM8 added a commit to TUM-Dev/Campus-UWP that referenced this issue Oct 28, 2018
@Tarunshrma
Copy link

Hi @Kipotlov ,

I am getting similar exception in my project too. I am getting it in iOS and Android Xamarin project. Do you any fix for that... it seems the issue is still open and your fix is not merged yet.

@COM8
Copy link
Author

COM8 commented Feb 18, 2019

To reduce the amount of exceptions I use the following as a workaround.

public int InsertOrReplace(object obj) {
    db.BeginTransaction();
    db.Delete(obj);
    int i = db.Insert(obj);
    db.Commit();
    return i;
}

Source: https://github.com/UWPX/UWPX-Client/blob/v.0.12.0.0/Thread_Save_Components/Classes/SQLite/TSSQLiteConnection.cs

Never the less it still throws an exception from time to time at db.Insert(obj);, but overall the exception count is significantly reduced by this.

@Tarunshrma
Copy link

To reduce the amount of exceptions I use the following as a workaround.

public int InsertOrReplace(object obj) {
    db.BeginTransaction();
    db.Delete(obj);
    int i = db.Insert(obj);
    db.Commit();
    return i;
}

Source: https://github.com/UWPX/UWPX-Client/blob/v.0.12.0.0/Thread_Save_Components/Classes/SQLite/TSSQLiteConnection.cs

Never the less it still throws an exception from time to time at db.Insert(obj);, but overall the exception count is significantly reduced by this.

Thanks a lot @COM8 I will try it out..

@bentmar
Copy link

bentmar commented Feb 22, 2019

I was trying to get the BUSY exception when building this, instead i get the excpetion youre talking about. It is always the task that started the first insert that fails. Im debugging on a physical device (android)

The version im using is 1.6.258-beta targetFramework="monoandroid90">

I got a repro, fails everytime! (almost)

 public InitStorage(IDatabaseConnectionProvider provider)
        {
            _dataConnection = provider.GetConnection(); // androiddb
           
            CreateAsync().GetAwaiter().GetResult(); // creates tables 

            Task.Run(() =>
            {
                SomeModel asd = new SomeModel();
                asd.Value = new string('A', 100000000);

                SomeModel asd2 = new SomeModel();
                asd2.Value = new string('A', 100000000);

                Task.Run(async () =>
                {
                    try
                    {
                        var a = await _dataConnection.InsertAsync(asd);
                        var a2 = await _dataConnection.InsertAsync(asd);
                    }
                    catch (Exception e)
                    {
                        var ascvbd = _dataConnection;
                        throw;
                    }
                });

                Task.Run(async() =>
                {
                    try
                    {
                        var b = await  _dataConnection.InsertAsync(asd2);
                        var b2 = await  _dataConnection.InsertAsync(asd2);
                    }
                    catch (Exception e)
                    {
                        var ascvbd = _dataConnection;
                        throw;
                    }
                });
            });
        }

@bentmar
Copy link

bentmar commented Feb 22, 2019

Now im wrapping every call in my sqlite repo with this https://alastaircrabtree.com/implementing-the-retry-pattern-for-async-tasks-in-c/ Works very good!

@COM8
Copy link
Author

COM8 commented Feb 22, 2019

@bentmar Looks good! Works for me too. Thanks!

COM8 added a commit to UWPX/UWPX-Client that referenced this issue Feb 22, 2019
@ElteHupkes
Copy link
Contributor

Glad I found this issue, I've been getting this randomly in our logs for a while. I think @Kipotlov indeed found the problem:

Ok, after a look in the code, i think i found the problem.
In this method, if added is false, prepCmd is disposed then return...

PreparedSqlLiteInsertCommand GetInsertCommand (TableMapping map, string extra)
{
	PreparedSqlLiteInsertCommand prepCmd;
	var key = Tuple.Create (map.MappedType.FullName, extra);

	lock (_insertCommandMap) {
		_insertCommandMap.TryGetValue (key, out prepCmd);
	}

	if (prepCmd == null) {
		prepCmd = CreateInsertCommand (map, extra);
		var added = false;
		lock (_insertCommandMap) {
			if (!_insertCommandMap.ContainsKey (key)) {
				_insertCommandMap.Add (key, prepCmd);
				added = true;
			}
		}
		if (!added) {
			prepCmd.Dispose ();
		}
	}

	return prepCmd;
}

Seems to me the issue would be resolved if instead of prepCmd we always return the value from the _insertCommandMap. I'll create a PR.

ElteHupkes added a commit to ElteHupkes/sqlite-net that referenced this issue Aug 29, 2019
This addresses a race condition where a new PreparedSqliteInsertCommand object was returned in a disposed state if a second object was created and inserted into the command map during its creation. The fix always returns the item inserted into the command map.
ElteHupkes added a commit to ElteHupkes/sqlite-net that referenced this issue Aug 29, 2019
@ElteHupkes
Copy link
Contributor

I've created #866, which includes a regression test (thanks @bentmar, indeed that code seems to trigger it almost always). Let's hope for a merge soon :).

@COM8
Copy link
Author

COM8 commented Sep 3, 2019

I think this won't happen 😉
Somebody any intrrest in helping maintain a fork that merges all the outstanding PRs?

@ElteHupkes
Copy link
Contributor

Hmyeah I see there's a lot of open PRs and not a lot of activity. Any reason for that @praeclarum? Not meant to be judgemental, we all have stuff to do beyond pleasing users of our unpaid open source contributions ;). It's just good to know, it would be better if there didn't need to be a fork.

praeclarum added a commit that referenced this issue Sep 20, 2019
Fix to #761, preventing returning a disposed command object
@praeclarum
Copy link
Owner

@ElteHupkes Please do not fork this thing :-) Already too many of those.

If I'm not being responsive to important bug fixes like this, harass me on Twitter.

Sorry for the delays, and thanks for the great work!

@ElteHupkes
Copy link
Contributor

Heheh my thoughts exactly. I'm not so active on Twitter myself but I'll keep that in mind ;). Thanks for merging!

github-actions bot pushed a commit to Reddevildragg-UPM-Forks/sqlite-net that referenced this issue Nov 17, 2020
This addresses a race condition where a new PreparedSqliteInsertCommand object was returned in a disposed state if a second object was created and inserted into the command map during its creation. The fix always returns the item inserted into the command map.
@madhuatlink
Copy link

@praeclarum can you please tell me whether this fix is available in the version '1.6.292'? Or do I need to update to '1.7.335' to get this issue fixed? Currently I am using 1.6.292 and I am getting this randomly. Thank you.

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

No branches or pull requests

7 participants