Move libuv calls to correct thread - #1197
johnhaley81 merged 5 commits into
Conversation
Sorry, something went wrong.
|
This was tested for stability in a staging GitKraken build, we didn't run into any problems. |
Sorry, something went wrong.
|
💯 Memory leak fixes and stability increases. 💯 @srajko talked offline about some naming issues that the PR brought out to the front. Namely that ExecuteAsync was very poorly named and lead to some confusion since it's not really "async" like JS async where you schedule a callback and return out of a function. ExecuteAsync schedules a callback and then in C++ land uses a semaphore to wait for a promise to resolve if the returned value was a promise. We both decided that we can update the names of the underlying callback system in a different PR and that this is GTG. |
Sorry, something went wrong.
Move libuv calls to correct thread
This PR removes uv_async_init / uv_close calls that were being called each time we were making callbacks from async threadpool threads into the main thread (for example, for clone progress callbacks). The change was prompted by Axosoft/nsfw#12 - in our case, the uv_async_init calls were being called from threadpool threads and that violated libuv's thread safety requirements.
The fix was to move the logic for these types of callbacks (I call them reverse callbacks in the code, since they are callbacks from the async threadpool code back into the main loop code) into our ThreadPool class, which now uses a single async handle and an internal queue to schedule these callbacks.
While at it, I also removed our last use of sleep_for_ms and replaced it with a semaphore.
We also had a return that would circumvent the uv_close -
nodegit/generate/templates/partials/field_accessors.cc
Line 142 in de3583b
Finally, the batons we were using for these reverse callbacks were dynamically allocated and never deallocated. I took out the dynamic allocation to avoid this problem.