Android-nRF-Mesh-Library: Potential ConcurrentException in MeshNetworkDb:InsertNetworkAsyncTask()

Hello, I’d like to report a second bug found during importing a MeshNetwork (The first one and solution has been post under BubblyNetDev post named “Migration issue in 3.1.5”, not sure if it has been read and is planned to be fixed in next releases).

By the way, In class MeshNetworkDb.Java

Problem :
In class :

private static class InsertNetworkAsyncTask extends AsyncTask<Void, Void, Void> {
    [...]
    if (!meshNetwork.nodes.isEmpty()) {
        nodesDao.insert(meshNetwork.nodes);
    }
 
    if (meshNetwork.groups != null) {
        groupsDao.insert(meshNetwork.groups);
    }
 
    if (meshNetwork.scenes != null) {
        scenesDao.insert(meshNetwork.scenes);
    }
    [...]
}

This can cause troubles down the line, in our case it caused several (still rare) ConcurrentExceptions :

 Fatal Exception: java.lang.RuntimeException: An error occurred while executing doInBackground()
        at android.os.AsyncTask$4.done(AsyncTask.java:415)
      ...
 Caused by java.util.ConcurrentModificationException
        at java.util.ArrayList$Itr.next(ArrayList.java:860)
        at androidx.room.EntityInsertionAdapter.insert(EntityInsertionAdapter.java:95)
        at no.nordicsemi.android.meshprovisioner.data.ProvisionedMeshNodesDao_Impl.insert(ProvisionedMeshNodesDao_Impl.java:303)
        at no.nordicsemi.android.meshprovisioner.MeshNetworkDb$InsertNetworkAsyncTask.doInBackground(MeshNetworkDb.java:307)
        at no.nordicsemi.android.meshprovisioner.MeshNetworkDb$InsertNetworkAsyncTask.doInBackground(MeshNetworkDb.java:271)

Cause : meshNetwork.nodes meshNetwork.groups and meshNetwork.scenes are arrayList. They can be modified from outside the MeshNetworkDb.java for any reason, it shouldn’t happen, but if it happens this leads to ConcurrentException.

Solution I fixed the problem cloning the arrayList before passing it to the Dao, this makes sure we never pass an array which could be potentially modified from outside:

    if (!meshNetwork.nodes.isEmpty()) {
        ArrayList<ProvisionedMeshNode> copyNodes = new ArrayList<>(meshNetwork.nodes);
        nodesDao.insert(copyNodes);
    }

About this issue

Original URL
State: closed
Created 3 years ago
Comments: 45 (23 by maintainers)

Commits related to this issue

Fixes #432 by passing an unmodifiable list for insert and update operations. — committed to NordicSemiconductor/Android-nRF-Mesh-Library by roshanrajaratnam 2 years ago
Fixes #432 network exclusions list must be unmodifiable. — committed to NordicSemiconductor/Android-nRF-Mesh-Library by roshanrajaratnam 2 years ago
Removes a multiple call to an internal delete function #432 — committed to NordicSemiconductor/Android-nRF-Mesh-Library by roshanrajaratnam 2 years ago
Contains a possible fix #432 by using an unmodifiable list when importing a mesh network. — committed to NordicSemiconductor/Android-nRF-Mesh-Library by roshanrajaratnam 2 years ago
Fix for #432 by returning a un-modifiable network exclusions list and fixes a possible concurrent modification exception when resetting a node. — committed to NordicSemiconductor/Android-nRF-Mesh-Library by roshanrajaratnam 2 years ago
Copies the network exclusion list to a new HashMap #432. — committed to NordicSemiconductor/Android-nRF-Mesh-Library by roshanrajaratnam 2 years ago

Most upvoted comments

I will use the approach that @Mavericksb has suggested since it’s a working solution. All updates and inserts will be done after copying the data in to a new list.

roshanrajaratnam on Feb 14, 2022

Any update on this issue? A few of our users are facing something quite similar: at no.nordicsemi.android.mesh.MeshNetworkDb.lambda$insertNetwork$0 (MeshNetworkDb.java:149)

What is the line of code at MeshNetworkDb.java:149 ? If there is an array passed to an insert() method, just copy the array first, and then pass the copy to the insert method instead of the original array, to avoid concurrency.

Mavericksb on Feb 7, 2022

We are hitting this repeatedly when making multiple changes to the network in quick succession

saty9 on Jun 10, 2021

Hi, thanks for reporting this. Asynctask has been deprecated in the library. However like you mentioned the migration may cause an issue. I will take a look.

roshanrajaratnam on May 25, 2021

Great, yes pull the latest Dev branch and you should get some other fix relating to an invalid 16-bit validation.

roshanrajaratnam on Mar 23, 2022

@FluffyBunniesTasteTheBest will take a look at this issue today and get back to you!

roshanrajaratnam on Mar 17, 2022

Any node that is reset gets added to the exclusion list. I still haven’t had time to test against your json file. I will push a fix, could you test it for me on your network?

roshanrajaratnam on Feb 25, 2022

@R0m4in-dooz could you export and share your network with me? I can test against your large exclusion list!

roshanrajaratnam on Feb 19, 2022

ok, I haven’t released yet. Please try it and let me know thanks.

roshanrajaratnam on Feb 17, 2022

Could you try this please?

roshanrajaratnam on Feb 17, 2022

Thanks for your feedback. You are correct I did forget to include network exclusions in the latest release.

roshanrajaratnam on Feb 17, 2022

Hello @roshanrajaratnam ! So I’ve pulled the new release to test the fixes, and it seems better, thanks for the update 🥳 Though the only place I flagged as “maybe our fault” is the one I can still get lol… Of course you didn’t apply the fix to the network exclusions list (by my fault?). But actually, I think it needs the fix too. (here)

V/MeshManagerApi(13645): Received network pdu: 0x0076CBD44F998565E7172186B5E3E4DC7C33FB2F5C
V/BaseMeshMessageHandler(13645): Sequence number of received Network PDU: 23
V/NetworkLayer(13645): TTL for received message: 127
V/NetworkLayer(13645): Dst: 0x499C
D/LowerTransportLayer(13645): IV Index of received message: 0
D/LowerTransportLayer(13645): SeqAuth: 23
V/AccessLayer(13645): Received Access PDU 804A
D/fr.dooz.nordic_nrf_mesh.DoozMeshManagerCallbacks(13645): onNetworkUpdated
D/fr.dooz.nordic_nrf_mesh.DoozMeshManagerCallbacks(13645): onNetworkUpdated
D/fr.dooz.nordic_nrf_mesh.DoozMeshManagerCallbacks(13645): onNetworkUpdated
D/fr.dooz.nordic_nrf_mesh.DoozMeshManagerCallbacks(13645): onNetworkUpdated
D/DoozMeshStatusCallbacks(13645): onMeshMessageReceived
D/DoozMeshStatusCallbacks(13645): received a ConfigNodeResetStatus
E/AndroidRuntime(13645): FATAL EXCEPTION: pool-21-thread-1
E/AndroidRuntime(13645): Process: fr.dooz.app, PID: 13645
E/AndroidRuntime(13645): java.util.ConcurrentModificationException
E/AndroidRuntime(13645): 	at java.util.ArrayList$Itr.next(ArrayList.java:860)
E/AndroidRuntime(13645): 	at com.google.gson.internal.bind.CollectionTypeAdapterFactory$Adapter.write(CollectionTypeAdapterFactory.java:96)
E/AndroidRuntime(13645): 	at com.google.gson.internal.bind.CollectionTypeAdapterFactory$Adapter.write(CollectionTypeAdapterFactory.java:61)
E/AndroidRuntime(13645): 	at com.google.gson.internal.bind.TypeAdapterRuntimeTypeWrapper.write(TypeAdapterRuntimeTypeWrapper.java:69)
E/AndroidRuntime(13645): 	at com.google.gson.internal.bind.MapTypeAdapterFactory$Adapter.write(MapTypeAdapterFactory.java:208)
E/AndroidRuntime(13645): 	at com.google.gson.internal.bind.MapTypeAdapterFactory$Adapter.write(MapTypeAdapterFactory.java:145)
E/AndroidRuntime(13645): 	at com.google.gson.Gson.toJson(Gson.java:735)
E/AndroidRuntime(13645): 	at com.google.gson.Gson.toJson(Gson.java:714)
E/AndroidRuntime(13645): 	at com.google.gson.Gson.toJson(Gson.java:669)
E/AndroidRuntime(13645): 	at com.google.gson.Gson.toJson(Gson.java:649)
E/AndroidRuntime(13645): 	at no.nordicsemi.android.mesh.MeshTypeConverters.networkExclusionsToJson(MeshTypeConverters.java:157)
E/AndroidRuntime(13645): 	at no.nordicsemi.android.mesh.MeshNetworkDb.lambda$update$3(MeshNetworkDb.java:200)
E/AndroidRuntime(13645): 	at no.nordicsemi.android.mesh.-$$Lambda$MeshNetworkDb$lIm-UMtxdFiQaNxV0ONcZDAI6eo.run(Unknown Source:4)
E/AndroidRuntime(13645): 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1162)
E/AndroidRuntime(13645): 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:636)
E/AndroidRuntime(13645): 	at java.lang.Thread.run(Thread.java:764)
E/AndroidRuntime(13645): FATAL EXCEPTION: pool-21-thread-2
E/AndroidRuntime(13645): Process: fr.dooz.app, PID: 13645
E/AndroidRuntime(13645): java.util.ConcurrentModificationException
E/AndroidRuntime(13645): 	at java.util.ArrayList$Itr.next(ArrayList.java:860)
E/AndroidRuntime(13645): 	at java.util.Collections$UnmodifiableCollection$1.next(Collections.java:1084)
E/AndroidRuntime(13645): 	at androidx.room.EntityDeletionOrUpdateAdapter.handleMultiple(EntityDeletionOrUpdateAdapter.java:86)
E/AndroidRuntime(13645): 	at no.nordicsemi.android.mesh.data.ProvisionedMeshNodesDao_Impl.update(ProvisionedMeshNodesDao_Impl.java:305)
E/AndroidRuntime(13645): 	at no.nordicsemi.android.mesh.MeshNetworkDb.lambda$update$6(MeshNetworkDb.java:227)
E/AndroidRuntime(13645): 	at no.nordicsemi.android.mesh.-$$Lambda$MeshNetworkDb$P_OWMUQ0TcsIRsw0eLJvpw-4dpE.run(Unknown Source:16)
E/AndroidRuntime(13645): 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1162)
E/AndroidRuntime(13645): 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:636)
E/AndroidRuntime(13645): 	at java.lang.Thread.run(Thread.java:764)

PS: I’m not getting it every time so it seems to be a sporadic issue 🤔 Now, most of the time I can receive ConfigNodeResetStatus without any issue, so again : thanks for the update 👍

R0m4in-dooz on Feb 17, 2022

Just for the records, there’s one more situation where the ConcurrentModificationException can occur and crash the app:

java.util.ConcurrentModificationException: 
  at java.util.ArrayList$Itr.next (ArrayList.java:860)
  at androidx.room.EntityDeletionOrUpdateAdapter.handleMultiple (EntityDeletionOrUpdateAdapter.java:86)
  at no.nordicsemi.android.mesh.data.ProvisionerDao_Impl.update (ProvisionerDao_Impl.java:232)
  at no.nordicsemi.android.mesh.MeshNetworkDb.lambda$update$16 (MeshNetworkDb.java:276)
  at no.nordicsemi.android.mesh.-$$Lambda$MeshNetworkDb$kyOqCg3Q_F167aHhj0mXWofVRzM.run (Unknown Source:4)
  at java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1167)
  at java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:641)
  at java.lang.Thread.run (Thread.java:920)

FluffyBunniesTasteTheBest on Feb 12, 2022

@Mavericksb Thank you very much for the clarification!

As far as I can tell you are trying to insert a Provisioner Node.

Yes. During startup, the app creates a bunch of provisioners, to enable multiple app instances to operate the mesh.

It’s hard to help without any more detail on the Table you are trying to write on, its definition, and the objects you are passing.

That’s all handled by the library, but…

SQLiteConstrainException isn’t related to the previous problem, it means that you are passing an Object (or list of Objects) that violate the constraints that are defined in Table constructor. … One of the most common causes (but it is just an example) could be that the Provisioner Table defined in MeshNetworkDb class needs an UNIQUE Key, and you are trying to insert an object (A Provisioner Node in this case) with an UNIQUE ID which already exists in the Table.

… that perfectly explains the cause of the SQLiteConstraintException:

If the app crashes because of a ConcurrentException, it tries to re-create the provisioners again during the next start, which, the way it looks, then causes SQLiteConstrainException because some of the provisioners already exist.

Thank you very much for sharing this! You’ve just saved me from quite some sleepless nights…

FluffyBunniesTasteTheBest on Feb 8, 2022

Beside of just copying the ArrayList, I’ve also delayed the call that’s causing the ConcurrentModificationException.

Ok, BTW adding a delay shouldn’t be necessary.

Google Play, btw., also logged some related SQLiteConstraintExceptions:

android.database.sqlite.SQLiteConstraintException: …

SQLiteConstrainException isn’t related to the previous problem, it means that you are passing an Object (or list of Objects) that violate the constraints that are defined in Table constructor. As far as I can tell you are trying to insert a Provisioner Node. It’s hard to help without any more detail on the Table you are trying to write on, its definition, and the objects you are passing. One of the most common causes (but it is just an example) could be that the Provisioner Table defined in MeshNetworkDb class needs an UNIQUE Key, and you are trying to insert an object (A Provisioner Node in this case) with an UNIQUE ID which already exists in the Table.

You need to do some debugging to find out where the problem lies. I suspect there is some circumstance where you don’t clear the Provisioner table before inserting a new Provisioner Node or you don’t change the Unique ID for the new one. Honestly I didn’t face it on our App, sorry I can’t help more than this.

Mavericksb on Feb 8, 2022