summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKonstantin Käfer <mail@kkaefer.com>2018-07-12 13:18:22 +0200
committerKonstantin Käfer <mail@kkaefer.com>2018-07-12 13:21:00 +0200
commitc939e0fb98e42bbf60e6f6bcd72d8675aa384bfa (patch)
tree713a5589427a856ace29c9869410c797961a98c3
parente16c96446584f8ee36da4722eb86f40b9cbe5659 (diff)
downloadqtlocation-mapboxgl-upstream/node-coalesce-cancel.tar.gz
[node] prevent scheduled renderFinished() calls from running when render is canceledupstream/node-coalesce-cancel
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.
-rw-r--r--platform/node/src/node_map.cpp25
1 files changed, 16 insertions, 9 deletions
diff --git a/platform/node/src/node_map.cpp b/platform/node/src/node_map.cpp
index 328bff1d7b..4bcd1d97bc 100644
--- a/platform/node/src/node_map.cpp
+++ b/platform/node/src/node_map.cpp
@@ -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);