diff options
author | Anand Thakker <anandthakker@users.noreply.github.com> | 2018-07-03 17:17:39 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-07-03 17:17:39 -0400 |
commit | cfd436c287f4209d0d994042452ccbb552a6bd28 (patch) | |
tree | 6811590928d7ea19db8e8b3f9db8d1df54ba9965 /src | |
parent | 840a5cf1207ed78df3302211a23d369dd3c12b89 (diff) | |
download | qtlocation-mapboxgl-cfd436c287f4209d0d994042452ccbb552a6bd28.tar.gz |
[core] Avoid blocking in Thread<Object> constructor (#12151)
* Introduce AspiringActor, EstablishedActor
This pair of objects represents the two-phase (parent-thread /
child-thread) construction that's needed to support constructing
Thread<Object> without blocking until the child thread is up and
running.
An `AspiringActor<O>` is responsible for:
- ownership of the actor's `Mailbox`
- allocating the memory for (but *not* constructing) the target object `O`
Using these two pieces--the mailbox and a stable address for `O`--an
`AspiringActor<O>` can accept messages for the target object, or provide
`ActorRef<O>`s that do so, before the object has actually been
constructed by the corresponding `EstablishedActor<O>`. (Such messages
are queued in the mailbox until after the object is constructed.)
This allows for an `AspiringActor<O>` to be created and safely used by a
thread other than the one on which the target object will (eventually)
live.
An `EstablishedActor<O>` is responsible for managing the lifetime of the
target object `O` and the open/closed state of the parent's `mailbox`.
The `O` object's lifetime is contained by that of its owning
`EstablishedActor<O>`: the `EstablishedActor` constructor executes the
`O` constructor via "placement new", constructing it at the address
provided by the parent `AspiringActor`, and the `~EstablishedActor`
destructor similarly executes the `~O` destructor (after closing the
mailbox). `EstablishedActor` should therefore live entirely on the
thread intended to own `O`.
* Remove Actor#{invoke,ask}
Diffstat (limited to 'src')
-rw-r--r-- | src/mbgl/actor/mailbox.cpp | 35 | ||||
-rw-r--r-- | src/mbgl/sprite/sprite_loader.cpp | 2 | ||||
-rw-r--r-- | src/mbgl/style/sources/custom_geometry_source.cpp | 6 | ||||
-rw-r--r-- | src/mbgl/tile/geometry_tile.cpp | 10 | ||||
-rw-r--r-- | src/mbgl/tile/raster_dem_tile.cpp | 2 | ||||
-rw-r--r-- | src/mbgl/tile/raster_tile.cpp | 2 |
6 files changed, 42 insertions, 15 deletions
diff --git a/src/mbgl/actor/mailbox.cpp b/src/mbgl/actor/mailbox.cpp index 373c24275f..8ee8dca114 100644 --- a/src/mbgl/actor/mailbox.cpp +++ b/src/mbgl/actor/mailbox.cpp @@ -6,8 +6,30 @@ namespace mbgl { +Mailbox::Mailbox() { +} + Mailbox::Mailbox(Scheduler& scheduler_) - : scheduler(scheduler_) { + : scheduler(&scheduler_) { +} + +void Mailbox::open(Scheduler& scheduler_) { + assert(!scheduler); + + // As with close(), block until neither receive() nor push() are in progress, and acquire the two + // mutexes in the same order. + std::lock_guard<std::recursive_mutex> receivingLock(receivingMutex); + std::lock_guard<std::mutex> pushingLock(pushingMutex); + + scheduler = &scheduler_; + + if (closed) { + return; + } + + if (!queue.empty()) { + (*scheduler)->schedule(shared_from_this()); + } } void Mailbox::close() { @@ -22,6 +44,9 @@ void Mailbox::close() { closed = true; } +bool Mailbox::isOpen() const { return bool(scheduler); } + + void Mailbox::push(std::unique_ptr<Message> message) { std::lock_guard<std::mutex> pushingLock(pushingMutex); @@ -32,13 +57,15 @@ void Mailbox::push(std::unique_ptr<Message> message) { std::lock_guard<std::mutex> queueLock(queueMutex); bool wasEmpty = queue.empty(); queue.push(std::move(message)); - if (wasEmpty) { - scheduler.schedule(shared_from_this()); + if (wasEmpty && scheduler) { + (*scheduler)->schedule(shared_from_this()); } } void Mailbox::receive() { std::lock_guard<std::recursive_mutex> receivingLock(receivingMutex); + + assert(scheduler); if (closed) { return; @@ -58,7 +85,7 @@ void Mailbox::receive() { (*message)(); if (!wasEmpty) { - scheduler.schedule(shared_from_this()); + (*scheduler)->schedule(shared_from_this()); } } diff --git a/src/mbgl/sprite/sprite_loader.cpp b/src/mbgl/sprite/sprite_loader.cpp index 93d6dfd9ae..df4fe6e8df 100644 --- a/src/mbgl/sprite/sprite_loader.cpp +++ b/src/mbgl/sprite/sprite_loader.cpp @@ -86,7 +86,7 @@ void SpriteLoader::emitSpriteLoadedIfComplete() { return; } - loader->worker.invoke(&SpriteLoaderWorker::parse, loader->image, loader->json); + loader->worker.self().invoke(&SpriteLoaderWorker::parse, loader->image, loader->json); } void SpriteLoader::onParsed(std::vector<std::unique_ptr<style::Image>>&& result) { diff --git a/src/mbgl/style/sources/custom_geometry_source.cpp b/src/mbgl/style/sources/custom_geometry_source.cpp index b37490a5ce..6ce7c1be11 100644 --- a/src/mbgl/style/sources/custom_geometry_source.cpp +++ b/src/mbgl/style/sources/custom_geometry_source.cpp @@ -30,15 +30,15 @@ void CustomGeometrySource::loadDescription(FileSource&) { void CustomGeometrySource::setTileData(const CanonicalTileID& tileID, const GeoJSON& data) { - loader->invoke(&CustomTileLoader::setTileData, tileID, data); + loader->self().invoke(&CustomTileLoader::setTileData, tileID, data); } void CustomGeometrySource::invalidateTile(const CanonicalTileID& tileID) { - loader->invoke(&CustomTileLoader::invalidateTile, tileID); + loader->self().invoke(&CustomTileLoader::invalidateTile, tileID); } void CustomGeometrySource::invalidateRegion(const LatLngBounds& bounds) { - loader->invoke(&CustomTileLoader::invalidateRegion, bounds, impl().getZoomRange()); + loader->self().invoke(&CustomTileLoader::invalidateRegion, bounds, impl().getZoomRange()); } } // namespace style diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp index af28fe3963..d686d8440b 100644 --- a/src/mbgl/tile/geometry_tile.cpp +++ b/src/mbgl/tile/geometry_tile.cpp @@ -86,7 +86,7 @@ void GeometryTile::setData(std::unique_ptr<const GeometryTileData> data_) { pending = true; ++correlationID; - worker.invoke(&GeometryTileWorker::setData, std::move(data_), correlationID); + worker.self().invoke(&GeometryTileWorker::setData, std::move(data_), correlationID); } @@ -112,14 +112,14 @@ void GeometryTile::setLayers(const std::vector<Immutable<Layer::Impl>>& layers) } ++correlationID; - worker.invoke(&GeometryTileWorker::setLayers, std::move(impls), correlationID); + worker.self().invoke(&GeometryTileWorker::setLayers, std::move(impls), correlationID); } void GeometryTile::setShowCollisionBoxes(const bool showCollisionBoxes_) { if (showCollisionBoxes != showCollisionBoxes_) { showCollisionBoxes = showCollisionBoxes_; ++correlationID; - worker.invoke(&GeometryTileWorker::setShowCollisionBoxes, showCollisionBoxes, correlationID); + worker.self().invoke(&GeometryTileWorker::setShowCollisionBoxes, showCollisionBoxes, correlationID); } } @@ -153,7 +153,7 @@ void GeometryTile::onError(std::exception_ptr err, const uint64_t resultCorrelat } void GeometryTile::onGlyphsAvailable(GlyphMap glyphs) { - worker.invoke(&GeometryTileWorker::onGlyphsAvailable, std::move(glyphs)); + worker.self().invoke(&GeometryTileWorker::onGlyphsAvailable, std::move(glyphs)); } void GeometryTile::getGlyphs(GlyphDependencies glyphDependencies) { @@ -161,7 +161,7 @@ void GeometryTile::getGlyphs(GlyphDependencies glyphDependencies) { } void GeometryTile::onImagesAvailable(ImageMap images, uint64_t imageCorrelationID) { - worker.invoke(&GeometryTileWorker::onImagesAvailable, std::move(images), imageCorrelationID); + worker.self().invoke(&GeometryTileWorker::onImagesAvailable, std::move(images), imageCorrelationID); } void GeometryTile::getImages(ImageRequestPair pair) { diff --git a/src/mbgl/tile/raster_dem_tile.cpp b/src/mbgl/tile/raster_dem_tile.cpp index 5db298cf4c..f29861ee71 100644 --- a/src/mbgl/tile/raster_dem_tile.cpp +++ b/src/mbgl/tile/raster_dem_tile.cpp @@ -48,7 +48,7 @@ void RasterDEMTile::setMetadata(optional<Timestamp> modified_, optional<Timestam void RasterDEMTile::setData(std::shared_ptr<const std::string> data) { pending = true; ++correlationID; - worker.invoke(&RasterDEMTileWorker::parse, data, correlationID, encoding); + worker.self().invoke(&RasterDEMTileWorker::parse, data, correlationID, encoding); } void RasterDEMTile::onParsed(std::unique_ptr<HillshadeBucket> result, const uint64_t resultCorrelationID) { diff --git a/src/mbgl/tile/raster_tile.cpp b/src/mbgl/tile/raster_tile.cpp index ff23d4493e..cc71c04ba1 100644 --- a/src/mbgl/tile/raster_tile.cpp +++ b/src/mbgl/tile/raster_tile.cpp @@ -37,7 +37,7 @@ void RasterTile::setMetadata(optional<Timestamp> modified_, optional<Timestamp> void RasterTile::setData(std::shared_ptr<const std::string> data) { pending = true; ++correlationID; - worker.invoke(&RasterTileWorker::parse, data, correlationID); + worker.self().invoke(&RasterTileWorker::parse, data, correlationID); } void RasterTile::onParsed(std::unique_ptr<RasterBucket> result, const uint64_t resultCorrelationID) { |