1

I am creating a Node addon to export a video file from macOS Photos library, as it takes a few seconds, I wrapped the code into an AsyncWorker.

The C++ / Objective-C code:

class Napi_PhotosExport_AsyncWorker : public Napi::AsyncWorker
{

  void Execute()
  {
    dispatch_group_t group = dispatch_group_create();
    dispatch_group_enter(group);

    [[PHImageManager defaultManager] requestExportSessionForVideo:... {

      // this should be called, but never be called.
      dispatch_group_leave(group);
    }];

    // this blocks the current thread and wait for the above callback.
    dispatch_group_wait(group, DISPATCH_TIME_FOREVER);
  }

};

The code above works in my macOS Cocoa application, but it doesn't work in Electron / Node environment for some reason. I expected the dispatch_group_leave can be called at some point, so that dispatch_group_wait returns properly instead of blocking the thread.

I am looking for some help from both macOS and Electron developers. Maybe Grand Central Dispatcher should not be used in Node addon, and I need to use C++ lock instead?

Hao Xi
  • 351
  • 4
  • 12

1 Answers1

0

The reason we always advise against this pattern is that:

  • if the closure is dispatched back to the same thread that is currently waiting, that can deadlock;

  • if you have thread explosion (either here or buried elsewhere in your project), this pattern can also deadlock; and

  • it is simply an inefficient anti-pattern that unnecessarily ties up threads, even if you do not introduce deadlocks.

As a rule, you simply never should be making asynchronous methods tie up threads to make them behave synchronously. Use asynchronous patterns; do not fight them.

BTW, the problem is almost certainly unrelated to the fact that you are using a dispatch group. Semaphores or locks are going to result in the same problem. The problem isn't how you're blocking the thread, but the fact that you are blocking it at all.

If you are doing this so that you can manage dependencies between asynchronous tasks, traditional solutions range from asynchronous NSOperation custom subclass, to third-party promises/futures library. In Swift, we would throw Combine and the async-await concurrency patterns into the discussion, too, but I'm assuming neither of those is an option for you. Needless to say, you could adopt traditional, less elegant patterns to solve this problem, such as recursion (initiating the next task in the completion of the prior one) or nested completion handlers (which, admittedly, can become unwieldy).

Rob
  • 415,655
  • 72
  • 787
  • 1,044
  • I can use the traditional way to pass a callback and to make it a promise style code, you are right, since it is a Node add on, introducing Swift would be too much, actually I am trying to not add any non-system frameworks. What I agree is, writing code in async way is better. What I don’t understand is, AsyncWorker was made for running code that could block the current thread, and it is not the main thread, so it won’t block Node’s event loop, how can it lead to dead lock… though it seems to be a dead lock, I can’t get it. Do you have any suggestions for me to debug my code? Thanks – Hao Xi Mar 02 '22 at 04:03
  • I removed `dispatch_group_wait` and pass a `resultHandler` into the method as a callback. It doesn't help, it seems when I call all those Cocoa APIs which has an async callback, the callback never execute for some reason. I guess it is something to do with Node threading model, instead of a dead lock. – Hao Xi Mar 02 '22 at 14:58
  • So, you’ve excised `wait` altogether and you’ve confirmed that it’s reaching this `requestExportSessionForVideo`, and it’s still not running the completion handler closure. Then you must have something else that is blocking the main thread (because this method’s completion handler is called on the main thread). – Rob Mar 02 '22 at 15:32
  • I did notice that these completion handlers were dispatched in main thread, I think you are right that something is blocking main thread, though I haven't figured out yet. When I test the addon, it is a simple index.js file with a readline at the end of the script, which keeps the process live so that async code can execute, maybe that's the problem. Meanwhile, I am facing a more critical issue that my addon crashes in Electron app, it did load and most of code is executed properly, but as soon as the addon tries to invoke any APIs in the Photos framework, the whole app crashed. – Hao Xi Mar 04 '22 at 14:14
  • Would you please take a look when you get a chance? Much appreciated. Here is the [link](https://stackoverflow.com/questions/71344200/node-addon-crashes-in-electron-but-works-fine-in-vanilla-node) – Hao Xi Mar 04 '22 at 14:20
  • Sorry, but I can't really help you on the Electron/Node stuff. I was only trying to illuminate why we never use the “block a thread and wait for a completion handler closure to be called” pattern. – Rob Mar 04 '22 at 23:19