diff options
author | John Firebaugh <john.firebaugh@gmail.com> | 2015-06-17 16:56:37 -0700 |
---|---|---|
committer | Thiago Marcos P. Santos <thiago@mapbox.com> | 2015-06-18 13:39:18 +0300 |
commit | eb52f86ee14c35d0af500cc82df0366f18ea20b7 (patch) | |
tree | e7727c25868917e3b295bd751c2429690a176abc /src | |
parent | 3ef8d8728e8da787484e49c2d082f7af63cfd717 (diff) | |
download | qtlocation-mapboxgl-eb52f86ee14c35d0af500cc82df0366f18ea20b7.tar.gz |
Fix an order-of-operations issue with tileLoadingCompleteCallback
==7466== Invalid read of size 8
==7466== at 0x5F1D0B: mbgl::TransformState::getAngle() const (transform_state.cpp:159)
==7466== by 0x700CD2: mbgl::Source::tileLoadingCompleteCallback(mbgl::TileID const&, mbgl::TransformState const&, bool) (source.cpp:574)
==7466== Address 0xf1d1508 is 56 bytes inside a block of size 120 free'd
==7466== at 0x4C2A4BC: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==7466== by 0x70AD61: std::_Function_base::_Base_manager<std::_Bind<std::_Mem_fn<void (mbgl::Source::*)(mbgl::TileID const&, mbgl::TransformState const&, bool)> ()(mbgl::Source*, mbgl::TileID, mbgl::TransformState, bool)> >::_M_destroy(std::_Any_data&, std::integral_constant<bool, false>) (functional:1894)
==7466== by 0x7080EB: std::_Function_base::_Base_manager<std::_Bind<std::_Mem_fn<void (mbgl::Source::*)(mbgl::TileID const&, mbgl::TransformState const&, bool)> ()(mbgl::Source*, mbgl::TileID, mbgl::TransformState, bool)> >::_M_manager(std::_Any_data&, std::_Any_data const&, std::_Manager_operation) (functional:1918)
==7466== by 0x535033: _ZNSt8functionIFvvEEaSEDn (functional:2277)
==7466== by 0x56A3F5: mbgl::WorkTask::cancel() (work_task.cpp:34)
==7466== by 0x56146C: mbgl::WorkRequest::~WorkRequest() (work_request.cpp:13)
==7466== by 0x50C5E0: std::default_delete<mbgl::WorkRequest>::operator()(mbgl::WorkRequest*) const (unique_ptr.h:76)
==7466== by 0x50C411: std::unique_ptr<mbgl::WorkRequest, std::default_delete<mbgl::WorkRequest> >::reset(mbgl::WorkRequest*) (unique_ptr.h:344)
==7466== by 0x50C374: std::unique_ptr<mbgl::WorkRequest, std::default_delete<mbgl::WorkRequest> >::operator=(std::unique_ptr<mbgl::WorkRequest, std::default_delete<mbgl::WorkRequest> >&&) (unique_ptr.h:251)
==7466== by 0x743136: mbgl::TileData::reparse(mbgl::Worker&, std::function<void ()()>) (tile_data.cpp:89)
==7466== by 0x6FEA7C: mbgl::Source::handlePartialTile(mbgl::TileID const&, mbgl::Worker&) (source.cpp:250)
==7466== by 0x700338: mbgl::Source::update(mbgl::MapData&, mbgl::TransformState const&, mbgl::Style&, mbgl::GlyphAtlas&, mbgl::GlyphStore&, mbgl::SpriteAtlas&, mbgl::util::ptr<mbgl::Sprite>, mbgl::TexturePool&, bool) (source.cpp:438)
==7466== by 0x68C1A5: mbgl::Style::update(mbgl::MapData&, mbgl::TransformState const&, mbgl::TexturePool&) (style.cpp:82)
==7466== by 0x548F5C: (anonymous namespace)::MockMapContext::update() (resource_loading.cpp:51)
==7466== by 0x548FA3: (anonymous namespace)::MockMapContext::onTileDataChanged() (resource_loading.cpp:56)
==7466== by 0x68CDE4: mbgl::Style::emitTileDataChanged() (style.cpp:212)
==7466== by 0x68CBDB: mbgl::Style::onTileLoaded(bool) (style.cpp:191)
==7466== by 0x700F38: mbgl::Source::emitTileLoaded(bool) (source.cpp:595)
==7466== by 0x700CA2: mbgl::Source::tileLoadingCompleteCallback(mbgl::TileID const&, mbgl::TransformState const&, bool) (source.cpp:573)
The invalid `TransformState::getAngle()` read comes from the following: the
storage for `TransformState` is the [bound parameter in this
callback](https://github.com/mapbox/mapbox-gl-native/blob/5e0775d276f9ea3652bae1
23c4e85cc05ae13bd5/src/mbgl/map/source.cpp#L292-292). The callback itself is
copied into various lambda bindings and eventually winds up as the [after
callback](https://github.com/mapbox/mapbox-gl-native/blob/5e0775d276f9ea3652bae1
23c4e85cc05ae13bd5/src/mbgl/map/tile_data.cpp#L89-89) for the worker task.
On the way out, `tileLoadingCompleteCallback` gets called via this callback
binding. It first does `emitTileLoaded`, which happens to cause [a _new_ work
request to be
queued](https://github.com/mapbox/mapbox-gl-native/blob/5e0775d276f9ea3652bae123
c4e85cc05ae13bd5/src/mbgl/map/source.cpp#L250-250). This cancels the
in-progress work request, [by nulling-out the `after`
callback](https://github.com/mapbox/mapbox-gl-native/blob/5e0775d276f9ea3652bae1
23c4e85cc05ae13bd5/src/mbgl/util/work_task.cpp#L34-34). But in fact, the after
callback is still executing. The next thing it does is [call
transformState.getAngle()](https://github.com/mapbox/mapbox-gl-native/blob/5e077
5d276f9ea3652bae123c4e85cc05ae13bd5/src/mbgl/map/source.cpp#L574), but the
transformState is now gone.
Diffstat (limited to 'src')
-rw-r--r-- | src/mbgl/map/source.cpp | 3 |
1 files changed, 1 insertions, 2 deletions
diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp index 663f1d3816..a45c371756 100644 --- a/src/mbgl/map/source.cpp +++ b/src/mbgl/map/source.cpp @@ -570,9 +570,8 @@ void Source::tileLoadingCompleteCallback(const TileID& normalized_id, const Tran return; } - emitTileLoaded(true); data->redoPlacement(transformState.getAngle(), collisionDebug); - + emitTileLoaded(true); } void Source::emitSourceLoaded() { |