[Discussion]Why not use nsoperation queue instead of dispatch queue in CDVCommandDelegateImpl.m?

[ECNU3D]

I'm using cordova 8.0.0 now to develop my hybrid application. I'm writing some plugin api function as followed

func pluginAPI( _ command: CDVInvokedUrlCommand ){
    // parameter process
    commandDelegate.run( inBackground: {() -> Void in
        var pluginResult : CDVPluginResult;
        // some logic to handle result
        self.commandDelegate.send( pluginResult, callbackId: command.callbackId );
    } );
}

In my scenario, as I'm delivering the plugin to different teams, I can't control how people will call it. In some cases, people will use some Promise.all in the front-end to call the pluginAPI function for more than 100 times.

Here brings the problem, I notice that the implementation of above run function is as followed:

- (void)runInBackground:(void (^)())block
{
    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), block);
}

It's using the GCD global concurrent queue directly, and I don't have an easy way to control the concurrent number of tasks. If the user calls the API more than 64 times(the maximum number of threads of GCD according to some documents), it will spawn up to 64 threads, and block other dispatch queue if I'm trying to dispatch any concurrent task.

For example, in my plugin, I've an array to store some data, which needs to be thread safe. So I implemented some read write lock according to the blog: http://basememara.com/creating-thread-safe-arrays-in-swift/ based on GCD.

let queue = DispatchQueue(label: "MyArrayQueue", attributes: .concurrent)
 
queue.async(flags: .barrier) {
  // Mutate array here
}
 
queue.sync() {
  // Read array here
}

In my code, I have some operation like

queue.sync()
queue.async()

The combination of queue.async/queue.sync call will actually create deadlock when pluginAPI calls already reach the maximum number of thread limit of GCD. It won't in normal cases.

I assume that it might be a common challenge in cordova plugin development, and I'm not sure what's the best practice here as we don't have an easy control of how javascript code calls the plugin API. One potential solution might be use the NSOperation to control the maximum concurrent number of tasks initiated by plugin API layer, which will leave the space for the following function in the execution chain to dispatch some async/sync to the queue. On the other hand, change the DispatchQueue from concurrent to serial will also fix the problem.

Any suggestions or ideas are welcomed!

[ECNU3D]

Attached the screenshot when deadlock happen, the SyncArraryQueue is the same as DispatchQueue mentioned above. The wrap_dispatch_sync is blocked by Disptachqueue.async as it is set as a barrier for SyncArraryQueue.

queue.async(flags: .barrier)

And the async task in SyncArraryQueue is pending as the GCD has reach the thread limitation. In normal case, the deadlock should not happen.

[ECNU3D]

The primary ask is to consider using NSOperation to have a limitation on the concurrency of the async plugin api calls instead of use dispatch queue directly and facing the potential abuse(calling it simultaneously for hundreds time) from the javascript side.

[ECNU3D]

Any maintainer could have a look at this issue？Or more input from myside is needed?@janpio

[brody4hire]

I suggest you send email to: [email protected]

You may want to try Slack #dev channel as well (link should be on cordova.io ).

Idea looks nice, sorry I have not much time to look in more detail. I think other members are pretty busy as well.

On Fri, Nov 9, 2018, 12:48 AM Tony Wang [email protected] wrote:

Any maintainer could have a look at this issue？Or more input from myside is needed?@janpio https://github.com/janpio

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/apache/cordova-ios/issues/452#issuecomment-437257412, or mute the thread https://github.com/notifications/unsubscribe-auth/ABfNUMla0Kmc3FFIwPKcWTGRYEnDy1gkks5utRcWgaJpZM4YN9YZ .

[oliversalzburg]

It sounds reasonable and the change seems minor, but I know too little of the subject to properly assess the change.

[brody4hire]

It would be great if someone could post a plugin that demonstrates this issue.

From some quick research I think this would be a nice enhancement for a major release, someday in the future. It should be pretty straightforward: https://stackoverflow.com/questions/39952743/how-to-use-nsoperation-nsoperationqueue-in-objective-c

I found another really nice resource at: https://cocoacasts.com/choosing-between-nsoperation-and-grand-central-dispatch/

A pull request would definitely be welcome.

[oliversalzburg]

@brodybits The PR is already at https://github.com/apache/cordova-ios/pull/454