Skip to content

Commit 248eb04

Browse files
louwerslouwers
authored
Revert "Fix segfault this-capture GeoJsonSource which may be deleted" maplibre#3536 (maplibre#3554)
1 parent 6e67ebf commit 248eb04

12 files changed

Lines changed: 53 additions & 63 deletions

File tree

‎platform/android/CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Changelog MapLibre Native for Android
22

3+
## 11.10.3
4+
5+
### 🐞 Bug fixes
6+
7+
- Revert fix [#3536](https://github.com/maplibre/maplibre-native/pull/3536) due to `getSource` crashes.
8+
39
## 11.10.2
410

511
### 🐞 Bug fixes

‎platform/android/MapLibreAndroid/src/cpp/style/sources/custom_geometry_source.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -141,18 +141,18 @@ void CustomGeometrySource::setTileData(
141141

142142
// Update the core source if not cancelled
143143
if (!isCancelled(z, x, y)) {
144-
getSource<mbgl::style::CustomGeometrySource>().CustomGeometrySource::setTileData(CanonicalTileID(z, x, y),
145-
GeoJSON(geometry));
144+
source.as<mbgl::style::CustomGeometrySource>()->CustomGeometrySource::setTileData(CanonicalTileID(z, x, y),
145+
GeoJSON(geometry));
146146
}
147147
}
148148

149149
void CustomGeometrySource::invalidateTile(jni::JNIEnv&, jni::jint z, jni::jint x, jni::jint y) {
150-
getSource<mbgl::style::CustomGeometrySource>().CustomGeometrySource::invalidateTile(CanonicalTileID(z, x, y));
150+
source.as<mbgl::style::CustomGeometrySource>()->CustomGeometrySource::invalidateTile(CanonicalTileID(z, x, y));
151151
}
152152

153153
void CustomGeometrySource::invalidateBounds(jni::JNIEnv& env, const jni::Object<LatLngBounds>& jBounds) {
154154
auto bounds = LatLngBounds::getLatLngBounds(env, jBounds);
155-
getSource<mbgl::style::CustomGeometrySource>().CustomGeometrySource::invalidateRegion(bounds);
155+
source.as<mbgl::style::CustomGeometrySource>()->CustomGeometrySource::invalidateRegion(bounds);
156156
}
157157

158158
jni::Local<jni::Array<jni::Object<geojson::Feature>>> CustomGeometrySource::querySourceFeatures(
@@ -162,7 +162,7 @@ jni::Local<jni::Array<jni::Object<geojson::Feature>>> CustomGeometrySource::quer
162162

163163
std::vector<mbgl::Feature> features;
164164
if (rendererFrontend) {
165-
features = rendererFrontend->querySourceFeatures(getSource().getID(), {{}, toFilter(env, jfilter)});
165+
features = rendererFrontend->querySourceFeatures(source.getID(), {{}, toFilter(env, jfilter)});
166166
}
167167
return Feature::convert(env, features);
168168
}

‎platform/android/MapLibreAndroid/src/cpp/style/sources/geojson_source.cpp‎

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,12 @@ GeoJSONSource::GeoJSONSource(jni::JNIEnv& env, const jni::String& sourceId, cons
5050
std::make_unique<mbgl::style::GeoJSONSource>(jni::Make<std::string>(env, sourceId),
5151
convertGeoJSONOptions(env, options))),
5252
converter(std::make_unique<Actor<FeatureConverter>>(Scheduler::GetBackground(),
53-
getSource<style::GeoJSONSource>().impl().getOptions())) {}
53+
source.as<style::GeoJSONSource>()->impl().getOptions())) {}
5454

5555
GeoJSONSource::GeoJSONSource(jni::JNIEnv& env, mbgl::style::Source& coreSource, AndroidRendererFrontend* frontend)
5656
: Source(env, coreSource, createJavaPeer(env), frontend),
5757
converter(std::make_unique<Actor<FeatureConverter>>(Scheduler::GetBackground(),
58-
getSource<style::GeoJSONSource>().impl().getOptions())) {}
58+
source.as<style::GeoJSONSource>()->impl().getOptions())) {}
5959

6060
GeoJSONSource::~GeoJSONSource() = default;
6161

@@ -83,11 +83,11 @@ void GeoJSONSource::setGeometry(jni::JNIEnv& env, const jni::Object<geojson::Geo
8383

8484
void GeoJSONSource::setURL(jni::JNIEnv& env, const jni::String& url) {
8585
// Update the core source
86-
getSource<style::GeoJSONSource>().setURL(jni::Make<std::string>(env, url));
86+
source.as<style::GeoJSONSource>()->setURL(jni::Make<std::string>(env, url));
8787
}
8888

8989
jni::Local<jni::String> GeoJSONSource::getURL(jni::JNIEnv& env) {
90-
std::optional<std::string> url = getSource<style::GeoJSONSource>().getURL();
90+
std::optional<std::string> url = source.as<style::GeoJSONSource>()->getURL();
9191
return url ? jni::Make<jni::String>(env, *url) : jni::Local<jni::String>();
9292
}
9393

@@ -98,7 +98,7 @@ jni::Local<jni::Array<jni::Object<geojson::Feature>>> GeoJSONSource::querySource
9898

9999
std::vector<mbgl::Feature> features;
100100
if (rendererFrontend) {
101-
features = rendererFrontend->querySourceFeatures(getSource().getID(), {{}, toFilter(env, jfilter)});
101+
features = rendererFrontend->querySourceFeatures(source.getID(), {{}, toFilter(env, jfilter)});
102102
}
103103
return Feature::convert(env, features);
104104
}
@@ -112,7 +112,7 @@ jni::Local<jni::Array<jni::Object<geojson::Feature>>> GeoJSONSource::getClusterC
112112
mbgl::Feature _feature = Feature::convert(env, feature);
113113
_feature.properties["cluster_id"] = static_cast<uint64_t>(_feature.properties["cluster_id"].get<double>());
114114
const auto featureExtension = rendererFrontend->queryFeatureExtensions(
115-
getSource().getID(), _feature, "supercluster", "children", {});
115+
source.getID(), _feature, "supercluster", "children", {});
116116
if (featureExtension.is<mbgl::FeatureCollection>()) {
117117
return Feature::convert(env, featureExtension.get<mbgl::FeatureCollection>());
118118
}
@@ -131,7 +131,7 @@ jni::Local<jni::Array<jni::Object<geojson::Feature>>> GeoJSONSource::getClusterL
131131
const std::map<std::string, mbgl::Value> options = {{"limit", static_cast<uint64_t>(limit)},
132132
{"offset", static_cast<uint64_t>(offset)}};
133133
auto featureExtension = rendererFrontend->queryFeatureExtensions(
134-
getSource().getID(), _feature, "supercluster", "leaves", options);
134+
source.getID(), _feature, "supercluster", "leaves", options);
135135
if (featureExtension.is<mbgl::FeatureCollection>()) {
136136
return Feature::convert(env, featureExtension.get<mbgl::FeatureCollection>());
137137
}
@@ -148,7 +148,7 @@ jint GeoJSONSource::getClusterExpansionZoom(jni::JNIEnv& env, const jni::Object<
148148
mbgl::Feature _feature = Feature::convert(env, feature);
149149
_feature.properties["cluster_id"] = static_cast<uint64_t>(_feature.properties["cluster_id"].get<double>());
150150
auto featureExtension = rendererFrontend->queryFeatureExtensions(
151-
getSource().getID(), _feature, "supercluster", "expansion-zoom", {});
151+
source.getID(), _feature, "supercluster", "expansion-zoom", {});
152152
if (featureExtension.is<mbgl::Value>()) {
153153
auto value = featureExtension.get<mbgl::Value>();
154154
if (value.is<uint64_t>()) {
@@ -181,20 +181,12 @@ void GeoJSONSource::setAsync(Update::Converter converterFn) {
181181
awaitingUpdate = std::make_unique<Update>(
182182
std::move(converterFn),
183183
std::make_unique<Actor<GeoJSONDataCallback>>(
184-
*Scheduler::GetCurrent(),
185-
[awaitingUpdateWeak = std::weak_ptr(awaitingUpdate),
186-
sourceWeak = getWeakSource(),
187-
updateWeak = std::weak_ptr(update)](std::shared_ptr<style::GeoJSONData> geoJSONData) {
188-
auto awaitingUpdate = awaitingUpdateWeak.lock();
189-
auto source = sourceWeak.lock();
190-
auto update = updateWeak.lock();
191-
if (!awaitingUpdate || !source || !update) return;
192-
184+
*Scheduler::GetCurrent(), [this](std::shared_ptr<style::GeoJSONData> geoJSONData) {
193185
// conversion from Java features to core ones finished
194186
android::UniqueEnv _env = android::AttachEnv();
195187

196188
// Update the core source
197-
source->get().as<style::GeoJSONSource>()->setGeoJSONData(std::move(geoJSONData));
189+
source.as<mbgl::style::GeoJSONSource>()->setGeoJSONData(std::move(geoJSONData));
198190

199191
// if there is an awaiting update, execute it, otherwise, release resources
200192
if (awaitingUpdate) {

‎platform/android/MapLibreAndroid/src/cpp/style/sources/geojson_source.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ class GeoJSONSource : public Source {
7070
jni::Local<jni::String> getURL(jni::JNIEnv&);
7171

7272
jni::Local<jni::Object<Source>> createJavaPeer(jni::JNIEnv&);
73-
std::shared_ptr<Update> awaitingUpdate;
74-
std::shared_ptr<Update> update;
73+
std::unique_ptr<Update> awaitingUpdate;
74+
std::unique_ptr<Update> update;
7575
std::shared_ptr<ThreadPool> threadPool;
7676
std::unique_ptr<Actor<FeatureConverter>> converter;
7777

‎platform/android/MapLibreAndroid/src/cpp/style/sources/image_source.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,20 +29,20 @@ ImageSource::~ImageSource() = default;
2929

3030
void ImageSource::setURL(jni::JNIEnv& env, const jni::String& url) {
3131
// Update the core source
32-
getSource<mbgl::style::ImageSource>().ImageSource::setURL(jni::Make<std::string>(env, url));
32+
source.as<mbgl::style::ImageSource>()->ImageSource::setURL(jni::Make<std::string>(env, url));
3333
}
3434

3535
jni::Local<jni::String> ImageSource::getURL(jni::JNIEnv& env) {
36-
std::optional<std::string> url = getSource<mbgl::style::ImageSource>().ImageSource::getURL();
36+
std::optional<std::string> url = source.as<mbgl::style::ImageSource>()->ImageSource::getURL();
3737
return url ? jni::Make<jni::String>(env, *url) : jni::Local<jni::String>();
3838
}
3939

4040
void ImageSource::setImage(jni::JNIEnv& env, const jni::Object<Bitmap>& bitmap) {
41-
getSource<mbgl::style::ImageSource>().setImage(Bitmap::GetImage(env, bitmap));
41+
source.as<mbgl::style::ImageSource>()->setImage(Bitmap::GetImage(env, bitmap));
4242
}
4343

4444
void ImageSource::setCoordinates(jni::JNIEnv& env, const jni::Object<LatLngQuad>& coordinatesObject) {
45-
getSource<mbgl::style::ImageSource>().setCoordinates(LatLngQuad::getLatLngArray(env, coordinatesObject));
45+
source.as<mbgl::style::ImageSource>()->setCoordinates(LatLngQuad::getLatLngArray(env, coordinatesObject));
4646
}
4747

4848
jni::Local<jni::Object<Source>> ImageSource::createJavaPeer(jni::JNIEnv& env) {

‎platform/android/MapLibreAndroid/src/cpp/style/sources/raster_dem_source.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ RasterDEMSource::RasterDEMSource(jni::JNIEnv& env, mbgl::style::Source& coreSour
2626
RasterDEMSource::~RasterDEMSource() = default;
2727

2828
jni::Local<jni::String> RasterDEMSource::getURL(jni::JNIEnv& env) {
29-
auto url = getSource<mbgl::style::RasterDEMSource>().RasterDEMSource::getURL();
29+
std::optional<std::string> url = source.as<mbgl::style::RasterDEMSource>()->RasterDEMSource::getURL();
3030
return url ? jni::Make<jni::String>(env, *url) : jni::Local<jni::String>();
3131
}
3232

‎platform/android/MapLibreAndroid/src/cpp/style/sources/raster_source.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ RasterSource::RasterSource(jni::JNIEnv& env, mbgl::style::Source& coreSource, An
2525
RasterSource::~RasterSource() = default;
2626

2727
jni::Local<jni::String> RasterSource::getURL(jni::JNIEnv& env) {
28-
auto url = getSource<mbgl::style::RasterSource>().RasterSource::getURL();
28+
std::optional<std::string> url = source.as<mbgl::style::RasterSource>()->RasterSource::getURL();
2929
return url ? jni::Make<jni::String>(env, *url) : jni::Local<jni::String>();
3030
}
3131

‎platform/android/MapLibreAndroid/src/cpp/style/sources/source.cpp‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,13 @@ Source::Source(jni::JNIEnv& env,
7070
mbgl::style::Source& coreSource,
7171
const jni::Object<Source>& obj,
7272
AndroidRendererFrontend* frontend)
73-
: source(std::make_shared<std::reference_wrapper<mbgl::style::Source>>(coreSource)),
73+
: source(coreSource),
7474
javaPeer(jni::NewGlobal(env, obj)),
7575
rendererFrontend(frontend) {}
7676

7777
Source::Source(jni::JNIEnv&, std::unique_ptr<mbgl::style::Source> coreSource)
7878
: ownedSource(std::move(coreSource)),
79-
source(std::make_shared<std::reference_wrapper<mbgl::style::Source>>(*ownedSource)) {}
79+
source(*ownedSource) {}
8080

8181
Source::~Source() {
8282
if (ownedSource) {
@@ -102,24 +102,24 @@ Source::~Source() {
102102
}
103103

104104
jni::Local<jni::String> Source::getId(jni::JNIEnv& env) {
105-
return jni::Make<jni::String>(env, source->get().getID());
105+
return jni::Make<jni::String>(env, source.getID());
106106
}
107107

108108
jni::Local<jni::String> Source::getAttribution(jni::JNIEnv& env) {
109-
auto attribution = source->get().getAttribution();
109+
auto attribution = source.getAttribution();
110110
return attribution ? jni::Make<jni::String>(env, attribution.value()) : jni::Make<jni::String>(env, "");
111111
}
112112

113113
void Source::setPrefetchZoomDelta(jni::JNIEnv& env, jni::Integer& delta) {
114114
if (!delta) {
115-
source->get().setPrefetchZoomDelta(std::nullopt);
115+
source.setPrefetchZoomDelta(std::nullopt);
116116
} else {
117-
source->get().setPrefetchZoomDelta(jni::Unbox(env, delta));
117+
source.setPrefetchZoomDelta(jni::Unbox(env, delta));
118118
}
119119
}
120120

121121
jni::Local<jni::Integer> Source::getPrefetchZoomDelta(jni::JNIEnv& env) {
122-
auto delta = source->get().getPrefetchZoomDelta();
122+
auto delta = source.getPrefetchZoomDelta();
123123
if (delta.has_value()) {
124124
return jni::Box(env, jni::jint(delta.value()));
125125
}
@@ -128,14 +128,14 @@ jni::Local<jni::Integer> Source::getPrefetchZoomDelta(jni::JNIEnv& env) {
128128

129129
void Source::setMaxOverscaleFactorForParentTiles(jni::JNIEnv& env, jni::Integer& maxOverscaleFactor) {
130130
if (!maxOverscaleFactor) {
131-
source->get().setMaxOverscaleFactorForParentTiles(std::nullopt);
131+
source.setMaxOverscaleFactorForParentTiles(std::nullopt);
132132
} else {
133-
source->get().setMaxOverscaleFactorForParentTiles(jni::Unbox(env, maxOverscaleFactor));
133+
source.setMaxOverscaleFactorForParentTiles(jni::Unbox(env, maxOverscaleFactor));
134134
}
135135
}
136136

137137
jni::Local<jni::Integer> Source::getMaxOverscaleFactorForParentTiles(jni::JNIEnv& env) {
138-
auto maxOverscaleFactor = source->get().getMaxOverscaleFactorForParentTiles();
138+
auto maxOverscaleFactor = source.getMaxOverscaleFactorForParentTiles();
139139
if (maxOverscaleFactor) {
140140
return jni::Box(env, jni::jint(*maxOverscaleFactor));
141141
}
@@ -151,7 +151,7 @@ void Source::addToStyle(JNIEnv& env, const jni::Object<Source>& obj, mbgl::style
151151
style.addSource(std::move(ownedSource));
152152

153153
// Add peer to core source
154-
source->get().peer = std::unique_ptr<Source>(this);
154+
source.peer = std::unique_ptr<Source>(this);
155155

156156
// Add strong reference to java source
157157
javaPeer = jni::NewGlobal(env, obj);
@@ -167,7 +167,7 @@ void Source::addToMap(JNIEnv& env, const jni::Object<Source>& obj, mbgl::Map& ma
167167
map.getStyle().addSource(std::move(ownedSource));
168168

169169
// Add peer to core source
170-
source->get().peer = std::unique_ptr<Source>(this);
170+
source.peer = std::unique_ptr<Source>(this);
171171

172172
// Add strong reference to java source
173173
javaPeer = jni::NewGlobal(env, obj);
@@ -182,26 +182,26 @@ bool Source::removeFromMap(JNIEnv&, const jni::Object<Source>&, mbgl::Map& map)
182182
}
183183

184184
// Remove the source from the map and take ownership
185-
ownedSource = map.getStyle().removeSource(source->get().getID());
185+
ownedSource = map.getStyle().removeSource(source.getID());
186186

187187
// The source may not be removed if any layers still reference it
188188
return ownedSource != nullptr;
189189
}
190190

191191
jni::Local<jni::Boolean> Source::isVolatile(jni::JNIEnv& env) {
192-
return jni::Box(env, jni::jboolean(source->get().isVolatile()));
192+
return jni::Box(env, jni::jboolean(source.isVolatile()));
193193
}
194194

195195
void Source::setVolatile(JNIEnv& env, jni::Boolean& value) {
196-
source->get().setVolatile(jni::Unbox(env, value));
196+
source.setVolatile(jni::Unbox(env, value));
197197
}
198198

199199
void Source::setMinimumTileUpdateInterval(JNIEnv& env, jni::Long& interval) {
200-
source->get().setMinimumTileUpdateInterval(Milliseconds(jni::Unbox(env, interval)));
200+
source.setMinimumTileUpdateInterval(Milliseconds(jni::Unbox(env, interval)));
201201
}
202202

203203
jni::Local<jni::Long> Source::getMinimumTileUpdateInterval(JNIEnv& env) {
204-
return jni::Box(env, jni::jlong(source->get().getMinimumTileUpdateInterval().count() / 1000000));
204+
return jni::Box(env, jni::jlong(source.getMinimumTileUpdateInterval().count() / 1000000));
205205
}
206206

207207
void Source::releaseJavaPeer() {

‎platform/android/MapLibreAndroid/src/cpp/style/sources/source.hpp‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
#include "../../android_renderer_frontend.hpp"
99

1010
#include <jni/jni.hpp>
11-
#include <functional>
1211

1312
namespace mbgl {
1413
namespace android {
@@ -64,21 +63,12 @@ class Source : private mbgl::util::noncopyable {
6463
jni::Local<jni::Long> getMinimumTileUpdateInterval(JNIEnv&);
6564

6665
protected:
67-
template <typename T = style::Source>
68-
T& getSource() {
69-
return *ownedSource->as<T>();
70-
}
71-
72-
std::weak_ptr<std::reference_wrapper<style::Source>> getWeakSource() const { return source; }
73-
74-
private:
7566
// Set on newly created sources until added to the map.
7667
std::unique_ptr<mbgl::style::Source> ownedSource;
7768

78-
// Reference that is valid at all times.
79-
std::shared_ptr<std::reference_wrapper<mbgl::style::Source>> source;
69+
// Raw pointer that is valid at all times.
70+
mbgl::style::Source& source;
8071

81-
protected:
8272
// Set when the source is added to a map.
8373
jni::Global<jni::Object<Source>> javaPeer;
8474

‎platform/android/MapLibreAndroid/src/cpp/style/sources/vector_source.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ VectorSource::VectorSource(jni::JNIEnv& env, mbgl::style::Source& coreSource, An
3131
VectorSource::~VectorSource() = default;
3232

3333
jni::Local<jni::String> VectorSource::getURL(jni::JNIEnv& env) {
34-
std::optional<std::string> url = getSource<mbgl::style::VectorSource>().VectorSource::getURL();
34+
std::optional<std::string> url = source.as<mbgl::style::VectorSource>()->VectorSource::getURL();
3535
return url ? jni::Make<jni::String>(env, *url) : jni::Local<jni::String>();
3636
}
3737

@@ -42,7 +42,7 @@ jni::Local<jni::Array<jni::Object<geojson::Feature>>> VectorSource::querySourceF
4242

4343
std::vector<mbgl::Feature> features;
4444
if (rendererFrontend) {
45-
features = rendererFrontend->querySourceFeatures(getSource().getID(),
45+
features = rendererFrontend->querySourceFeatures(source.getID(),
4646
{toVector(env, jSourceLayerIds), toFilter(env, jfilter)});
4747
}
4848
return Feature::convert(env, features);

0 commit comments

Comments
 (0)