summaryrefslogtreecommitdiff
path: root/src/mbgl/map
diff options
context:
space:
mode:
authorJulian Rex <julian.rex@gmail.com>2018-05-21 14:14:58 -0400
committerGitHub <noreply@github.com>2018-05-21 14:14:58 -0400
commitbef2a7ccb6e800d2ca6f2bac5c863b49fbba103d (patch)
treec64dc6e8130425b3b4131cd2bc405dee37224de7 /src/mbgl/map
parenta84ac4c8d79952fa3031f5414b10a560fdef2e1d (diff)
downloadqtlocation-mapboxgl-bef2a7ccb6e800d2ca6f2bac5c863b49fbba103d.tar.gz
[ios,macos] Fix for infinite loop crash when setting shape to nil in MGLMapViewDelegate methods (#11614)
Diffstat (limited to 'src/mbgl/map')
-rw-r--r--src/mbgl/map/transform.cpp57
-rw-r--r--src/mbgl/map/transform.hpp2
2 files changed, 49 insertions, 10 deletions
diff --git a/src/mbgl/map/transform.cpp b/src/mbgl/map/transform.cpp
index 105adf0400..da8e243d91 100644
--- a/src/mbgl/map/transform.cpp
+++ b/src/mbgl/map/transform.cpp
@@ -594,13 +594,10 @@ void Transform::startTransition(const CameraOptions& camera,
animation.transitionFrameFn(t);
}
observer.onCameraIsChanging();
+ return false;
} else {
- transitionFinishFn();
- transitionFinishFn = nullptr;
-
- // This callback gets destroyed here,
- // we can only return after this point.
- transitionFrameFn = nullptr;
+ // Indicate that we need to terminate this transition
+ return true;
}
};
@@ -615,7 +612,14 @@ void Transform::startTransition(const CameraOptions& camera,
};
if (!isAnimated) {
- transitionFrameFn(Clock::now());
+ auto update = std::move(transitionFrameFn);
+ auto finish = std::move(transitionFinishFn);
+
+ transitionFrameFn = nullptr;
+ transitionFinishFn = nullptr;
+
+ update(Clock::now());
+ finish();
}
}
@@ -624,8 +628,43 @@ bool Transform::inTransition() const {
}
void Transform::updateTransitions(const TimePoint& now) {
- if (transitionFrameFn) {
- transitionFrameFn(now);
+
+ // Use a temporary function to ensure that the transitionFrameFn lambda is
+ // called only once per update.
+
+ // This addresses the symptoms of https://github.com/mapbox/mapbox-gl-native/issues/11180
+ // where setting a shape source to nil (or similar) in the `onCameraIsChanging`
+ // observer function causes `Map::Impl::onUpdate()` to be called which
+ // in turn calls this function (before the current iteration has completed),
+ // leading to an infinite loop. See https://github.com/mapbox/mapbox-gl-native/issues/5833
+ // for a similar, related, issue.
+ //
+ // By temporarily nulling the `transitionFrameFn` (and then restoring it
+ // after the temporary has been called) we stop this recursion.
+ //
+ // It's important to note that the scope of this change is stop the above
+ // crashes. It doesn't address any potential deeper issue (for example
+ // user error, how often and when transition callbacks are called).
+
+ auto transition = std::move(transitionFrameFn);
+ transitionFrameFn = nullptr;
+
+ if (transition && transition(now)) {
+ // If the transition indicates that it is complete, then we should call
+ // the finish lambda (going via a temporary as above)
+ auto finish = std::move(transitionFinishFn);
+
+ transitionFinishFn = nullptr;
+ transitionFrameFn = nullptr;
+
+ if (finish) {
+ finish();
+ }
+ } else if (!transitionFrameFn) {
+ // We have to check `transitionFrameFn` is nil here, since a new transition
+ // may have been triggered in a user callback (from the transition call
+ // above)
+ transitionFrameFn = std::move(transition);
}
}
diff --git a/src/mbgl/map/transform.hpp b/src/mbgl/map/transform.hpp
index d429c57661..bff44a2dcd 100644
--- a/src/mbgl/map/transform.hpp
+++ b/src/mbgl/map/transform.hpp
@@ -165,7 +165,7 @@ private:
TimePoint transitionStart;
Duration transitionDuration;
- std::function<void(const TimePoint)> transitionFrameFn;
+ std::function<bool(const TimePoint)> transitionFrameFn;
std::function<void()> transitionFinishFn;
};