Is the framework threadsafe?

[eanderlind]

Is the nRFProvision framework threadsafe? It appears writes to the MeshNetwork class all occur from the
main thread (mostly done by view controllers, see below), however reads also occur from the background threadpool threads that serve NetworkManager and it's "meshstack" layers.
E.g. the UpperTransportPDU.decode() method accesses the meshnetwork.groups, meshnetwork.applicationkeys and meshnetwork.nodes arrays. Is this a correct understanding, or is there some other closure capture mechansism in play that mitigates this when work is run from the background dispatch queue?

I see the data structures internal to the layers (e.g. UpperTransportLayer.queues) are properly protected by a lock. It appears with the Example App UI the probabiliity for memory access conflicts is low, due to time it takes for a user to navigate views, although it could happen if sending e.g. Config msgs to a node several hops away, or if a 2nd Provisioner client is also configuring a different node.

I am aware of the MeshNetworkManager.instance() method that switches to main thread before accessing meshnetwork. The model delegates use this mechanism when accessing meshnetwork object and processing received msgs. The viewcontrollers also access through this method, but threafter appear to make assumption it is on main thread. Is it possible to redefine .instance() so that it instead accesses meshnetwork objects on a custom thread pool (and modifying UI accordingly, or accept frequent GCD priority inversion when accessed from main thread)?

[philips77]

Hi, sorry for the long delay.
I hope I'll resume working in this library soon.
Indeed, the app does it all from the main thread, but i was asked to add some automation (to make the initial configuration process smoother), do this will force me to look into that.
Is giving a dispatch queue like for delegates good option for you?

[philips77]

Hi,
A year has passed, so it's time to reply. The reason why .instance() is using main thread is because it accesses the UIApplication.shared.delegate. The manager should be thread safe. I'll try to implement suggested migration to a custom thread.

[philips77]

Also, the UI is modified on the queue given in the MeshNetworkManager initializer. By default it's the main thread, but could be a different if needed.

[eanderlind]

I scanned the framework layers and only potential concern is

IOS-nRF-Mesh-Library/nRFMeshProvision/Layers/Access Layer/AccessLayer.swift

Line 170 in 514587b

let index = mutex.sync(execute: {

where code grabs an array index in one sync call and uses it in a 2nd.
Great to see all the code improvements over past year.

We defaulted to always mutate MeshNetworkManager from main thread following convention in Example App. One downside is there is a lot of main thread processing when saving. I don't think access to this class can be moved to another thread (at least yet). Issue title may be missleading since it is a shared assumption between Example App and framework that any mutation occurs on main thread. No problem if you want to close this issue and track any remaining efforts differently.

[philips77]

I will release the lib as is now and will look into that later. And let's leave it open not to forget.

Thanks for the feedback! Hope you don't have problems migrating to the latest changes.