[node] prevent scheduled renderFinished() calls from running when render is canceled
Reverts d5aff71 (#11948) and implements an alternative solution.
Even with the previous fix applied, we were calling `renderFinished()` directly when the render was canceled. This led to the following scenario (all on the main thread):
- `Map::renderStill()` callback is called, `uv_async_send` schedules a call to `renderFinished()`.
- `NodeMap::cancel()` destroys the map object, and calls `renderFinished()` directly, which cleans up the request
- `NodeMap::startRender()` is called to schedule a _new_ render, which means that a new callback will be set
- The `uv_async_t` fires and calls `renderFinished` again:
- We don't return early as intended by the fix in #11948 since a new callback/`req` object is now available.
- Since neither error nor image has been set, we'll see a `Didn't get an image` error message.
- We call the callback with the error. In scenario where we're reusing Map objects, we will start a new render, which will now trigger the `Map is currently rendering an image` error in `Map::renderStill`, and the map object enters a circle in which it will now fail repeatedly with this error.
This commit fixes the issue by completely removing the existing `uv_async_t` handle that could have a scheduled call, and replacing it with a fresh instance of an `uv_async_t` handle.
This commit is contained in:
@@ -460,21 +460,13 @@ void NodeMap::startRender(NodeMap::RenderOptions options) {
|
||||
}
|
||||
|
||||
void NodeMap::renderFinished() {
|
||||
if (!req) {
|
||||
// In some situations, the render finishes at the same time as we call cancel. Make sure
|
||||
// we are only finishing a render once.
|
||||
return;
|
||||
}
|
||||
assert(req);
|
||||
|
||||
Nan::HandleScope scope;
|
||||
|
||||
// We're done with this render call, so we're unrefing so that the loop could close.
|
||||
uv_unref(reinterpret_cast<uv_handle_t *>(async));
|
||||
|
||||
// There is no render pending anymore, we the GC could now delete this object if it went out
|
||||
// of scope.
|
||||
Unref();
|
||||
|
||||
// Move the callback and image out of the way so that the callback can start a new render call.
|
||||
auto request = std::move(req);
|
||||
auto img = std::move(image);
|
||||
@@ -527,6 +519,10 @@ void NodeMap::renderFinished() {
|
||||
};
|
||||
request->runInAsyncScope(target, callback, 1, argv);
|
||||
}
|
||||
|
||||
// There is no render pending anymore, we the GC could now delete this object if it went out
|
||||
// of scope.
|
||||
Unref();
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -585,6 +581,17 @@ void NodeMap::cancel() {
|
||||
// Reset map explicitly as it resets the renderer frontend
|
||||
map.reset();
|
||||
|
||||
// Remove the existing async handle to flush any scheduled calls to renderFinished.
|
||||
uv_unref(reinterpret_cast<uv_handle_t *>(async));
|
||||
uv_close(reinterpret_cast<uv_handle_t *>(async), [] (uv_handle_t *h) {
|
||||
delete reinterpret_cast<uv_async_t *>(h);
|
||||
});
|
||||
async = new uv_async_t;
|
||||
async->data = this;
|
||||
uv_async_init(uv_default_loop(), async, [](uv_async_t* h) {
|
||||
reinterpret_cast<NodeMap *>(h->data)->renderFinished();
|
||||
});
|
||||
|
||||
frontend = std::make_unique<mbgl::HeadlessFrontend>(mbgl::Size{ 256, 256 }, pixelRatio, *this, threadpool);
|
||||
map = std::make_unique<mbgl::Map>(*frontend, mapObserver, frontend->getSize(), pixelRatio,
|
||||
*this, threadpool, mode);
|
||||
|
||||
Reference in New Issue
Block a user