diff --git a/clip.cpp b/clip.cpp index a50a7eda..72a1fb3a 100644 --- a/clip.cpp +++ b/clip.cpp @@ -1221,7 +1221,7 @@ std::string overzoom(std::vector const &tiles, int nz, int nx, int n std::vector const &exclude_prefix, bool do_compress, std::vector> *next_overzoomed_tiles, - bool demultiply, json_object_ptr filter, bool preserve_input_order, + bool demultiply, json_object *filter, bool preserve_input_order, std::unordered_map const &attribute_accum, std::vector const &unidecode_data, double simplification, double tiny_polygon_size, @@ -1457,7 +1457,7 @@ std::string overzoom(std::vector const &tiles, int nz, int nx, int std::vector const &exclude_prefix, bool do_compress, std::vector> *next_overzoomed_tiles, - bool demultiply, json_object_ptr filter, bool preserve_input_order, + bool demultiply, json_object *filter, bool preserve_input_order, std::unordered_map const &attribute_accum, std::vector const &unidecode_data, double simplification, double tiny_polygon_size, diff --git a/evaluator.cpp b/evaluator.cpp index efd27746..ce1e576c 100644 --- a/evaluator.cpp +++ b/evaluator.cpp @@ -9,7 +9,7 @@ #include "milo/dtoa_milo.h" #include "text.hpp" -int compare(mvt_value const &one, json_object_ptr two, bool &fail) { +int compare(mvt_value const &one, json_object *two, bool &fail) { switch (one.type) { case mvt_string: if (two->type != JSON_STRING) { @@ -91,7 +91,7 @@ int compare(mvt_value const &one, json_object_ptr two, bool &fail) { // 0: false // 1: true // -1: incomparable (sql null), treated as false in final output -static int eval(std::function feature, json_object_ptr f, std::set &exclude_attributes, std::vector const &unidecode_data) { +static int eval(std::function feature, json_object *f, std::set &exclude_attributes, std::vector const &unidecode_data) { if (f != nullptr) { if (f->type == JSON_TRUE) { return 1; @@ -188,7 +188,7 @@ static int eval(std::function feature, json_obje } bool fail = false; - int cmp = compare(ff, f->array()[2], fail); + int cmp = compare(ff, f->array()[2].get(), fail); if (fail) { static bool warned = false; @@ -237,7 +237,7 @@ static int eval(std::function feature, json_obje } for (size_t i = 1; i < f->array().size(); i++) { - int out = eval(feature, f->array()[i], exclude_attributes, unidecode_data); + int out = eval(feature, f->array()[i].get(), exclude_attributes, unidecode_data); if (out >= 0) { // nulls are ignored in boolean and/or expressions if (op == "all") { @@ -289,7 +289,7 @@ static int eval(std::function feature, json_obje bool found = false; for (size_t i = 2; i < f->array().size(); i++) { bool fail = false; - int cmp = compare(ff, f->array()[i], fail); + int cmp = compare(ff, f->array()[i].get(), fail); if (fail) { static bool warned = false; @@ -324,7 +324,7 @@ static int eval(std::function feature, json_obje exit(EXIT_FILTER); } - bool ok = eval(feature, f->array()[2], exclude_attributes, unidecode_data) > 0; + bool ok = eval(feature, f->array()[2].get(), exclude_attributes, unidecode_data) > 0; if (!ok) { exclude_attributes.insert(f->array()[1]->string()); } @@ -336,14 +336,14 @@ static int eval(std::function feature, json_obje exit(EXIT_FILTER); } -bool evaluate(std::function feature, std::string const &layer, json_object_ptr filter, std::set &exclude_attributes, std::vector const &unidecode_data) { +static bool evaluate(std::function feature, std::string const &layer, json_object *filter, std::set &exclude_attributes, std::vector const &unidecode_data) { if (filter == nullptr || filter->type != JSON_HASH) { fprintf(stderr, "Error: filter is not a hash: %s\n", json_stringify(filter).c_str()); exit(EXIT_JSON); } bool ok = true; - json_object_ptr f; + json_object *f; f = json_hash_get(filter, layer.c_str()); if (ok && f != nullptr) { @@ -371,7 +371,6 @@ json_object_ptr read_filter(const char *fname) { fprintf(stderr, "%s: %s\n", fname, jp->error); exit(EXIT_JSON); } - json_disconnect(filter); fclose(fp); return filter; } @@ -384,11 +383,10 @@ json_object_ptr parse_filter(const char *s) { fprintf(stderr, "%s\n", jp->error); exit(EXIT_JSON); } - json_disconnect(filter); return filter; } -bool evaluate(std::unordered_map const &feature, std::string const &layer, json_object_ptr filter, std::set &exclude_attributes, std::vector const &unidecode_data) { +bool evaluate(std::unordered_map const &feature, std::string const &layer, json_object *filter, std::set &exclude_attributes, std::vector const &unidecode_data) { std::function getter = [&](std::string const &key) { auto f = feature.find(key); if (f != feature.end()) { @@ -404,7 +402,7 @@ bool evaluate(std::unordered_map const &feature, std::st return evaluate(getter, layer, filter, exclude_attributes, unidecode_data); } -bool evaluate(mvt_feature const &feat, mvt_layer const &layer, json_object_ptr filter, std::set &exclude_attributes, int z, std::vector const &unidecode_data) { +bool evaluate(mvt_feature const &feat, mvt_layer const &layer, json_object *filter, std::set &exclude_attributes, int z, std::vector const &unidecode_data) { std::function getter = [&](std::string const &key) { const static std::string dollar_id = "$id"; if (key == dollar_id && feat.has_id) { diff --git a/evaluator.hpp b/evaluator.hpp index 99a91fb7..a2c234fb 100644 --- a/evaluator.hpp +++ b/evaluator.hpp @@ -7,10 +7,14 @@ #include "jsonpull/jsonpull.h" #include "mvt.hpp" -bool evaluate(std::unordered_map const &feature, std::string const &layer, json_object_ptr filter, std::set &exclude_attributes, std::vector const &unidecode_data); +// The `filter` parameters take a borrowed pointer; the caller (in +// main.cpp, tile-join, overzoom) keeps the json_object_ptr alive +// across every per-feature evaluate() call. A raw pointer avoids +// touching unique_ptr at all on this hot path. +bool evaluate(std::unordered_map const &feature, std::string const &layer, json_object *filter, std::set &exclude_attributes, std::vector const &unidecode_data); json_object_ptr parse_filter(const char *s); json_object_ptr read_filter(const char *fname); -bool evaluate(mvt_feature const &feat, mvt_layer const &layer, json_object_ptr filter, std::set &exclude_attributes, int z, std::vector const &unidecode_data); +bool evaluate(mvt_feature const &feat, mvt_layer const &layer, json_object *filter, std::set &exclude_attributes, int z, std::vector const &unidecode_data); #endif diff --git a/geobuf.cpp b/geobuf.cpp index 721c675c..6481695e 100644 --- a/geobuf.cpp +++ b/geobuf.cpp @@ -398,17 +398,17 @@ void readFeature(protozero::pbf_reader &pbf, size_t dim, double e, std::vectortype == JSON_NUMBER)) { sf.tippecanoe_minzoom = integer_zoom(sst->fname, milo::dtoa_milo(min->number())); } - json_object_ptr max = json_hash_get(o, "maxzoom"); + json_object *max = json_hash_get(o, "maxzoom"); if (max != nullptr && (max->type == JSON_NUMBER)) { sf.tippecanoe_maxzoom = integer_zoom(sst->fname, milo::dtoa_milo(max->number())); } - json_object_ptr tlayer = json_hash_get(o, "layer"); + json_object *tlayer = json_hash_get(o, "layer"); if (tlayer != nullptr && (tlayer->type == JSON_STRING)) { layername = tlayer->string(); } diff --git a/geojson-loop.cpp b/geojson-loop.cpp index 75149e94..e4f2f40b 100644 --- a/geojson-loop.cpp +++ b/geojson-loop.cpp @@ -25,7 +25,7 @@ static const char *geometry_names[GEOM_TYPES] = { }; // XXX duplicated -static void json_context(json_object_ptr j) { +static void json_context(json_object *j) { std::string s = json_stringify(j); if (s.size() >= 500) { @@ -36,18 +36,18 @@ static void json_context(json_object_ptr j) { fprintf(stderr, "in JSON object %s\n", s.c_str()); } -void parse_json(json_feature_action *jfa, json_pull_ptr jp) { +void parse_json(json_feature_action *jfa, json_pull_ptr &jp) { long long found_hashes = 0; long long found_features = 0; long long found_geometries = 0; while (1) { - json_object_ptr j = json_read(jp); + json_object *j = json_read(jp); if (j == nullptr) { if (jp->error != nullptr) { fprintf(stderr, "%s:%d: %s: ", jfa->fname.c_str(), jp->line, jp->error); if (jp->root != nullptr) { - json_context(jp->root); + json_context(jp->root.get()); } else { fprintf(stderr, "\n"); } @@ -65,7 +65,7 @@ void parse_json(json_feature_action *jfa, json_pull_ptr jp) { } } - json_object_ptr type = json_hash_get(j, "type"); + json_object *type = json_hash_get(j, "type"); if (type == nullptr || type->type != JSON_STRING) { continue; } @@ -84,14 +84,14 @@ void parse_json(json_feature_action *jfa, json_pull_ptr jp) { if (j->parent != nullptr) { if (j->parent->type == JSON_ARRAY && j->parent->parent != nullptr) { if (j->parent->parent->type == JSON_HASH) { - json_object_ptr geometries = json_hash_get(j->parent->parent, "geometries"); + json_object *geometries = json_hash_get(j->parent->parent, "geometries"); if (geometries != nullptr) { // Parent of Parent must be a GeometryCollection is_geometry = 0; } } } else if (j->parent->type == JSON_HASH) { - json_object_ptr geometry = json_hash_get(j->parent, "geometry"); + json_object *geometry = json_hash_get(j->parent, "geometry"); if (geometry != nullptr) { // Parent must be a Feature is_geometry = 0; @@ -101,10 +101,10 @@ void parse_json(json_feature_action *jfa, json_pull_ptr jp) { } if (is_geometry) { - json_object *jo = j.get(); + json_object *jo = j; while (jo != nullptr) { if (jo->parent != nullptr && jo->parent->type == JSON_HASH) { - if (json_hash_get(jo->parent, "properties").get() == jo) { + if (json_hash_get(jo->parent, "properties") == jo) { // Ancestor is the value corresponding to a properties key is_geometry = 0; break; @@ -140,7 +140,7 @@ void parse_json(json_feature_action *jfa, json_pull_ptr jp) { } found_features++; - json_object_ptr geometry = json_hash_get(j, "geometry"); + json_object *geometry = json_hash_get(j, "geometry"); if (geometry == nullptr) { fprintf(stderr, "%s:%d: feature with no geometry: ", jfa->fname.c_str(), jp->line); json_context(j); @@ -148,7 +148,7 @@ void parse_json(json_feature_action *jfa, json_pull_ptr jp) { continue; } - json_object_ptr properties = json_hash_get(j, "properties"); + json_object *properties = json_hash_get(j, "properties"); if (properties == nullptr || (properties->type != JSON_HASH && properties->type != JSON_NULL)) { fprintf(stderr, "%s:%d: feature without properties hash: ", jfa->fname.c_str(), jp->line); json_context(j); @@ -158,10 +158,10 @@ void parse_json(json_feature_action *jfa, json_pull_ptr jp) { bool is_feature = true; { - json_object *jo = j.get(); + json_object *jo = j; while (jo != nullptr) { if (jo->parent != nullptr && jo->parent->type == JSON_HASH) { - if (json_hash_get(jo->parent, "properties").get() == jo) { + if (json_hash_get(jo->parent, "properties") == jo) { // Ancestor is the value corresponding to a properties key is_feature = false; break; @@ -174,10 +174,10 @@ void parse_json(json_feature_action *jfa, json_pull_ptr jp) { continue; } - json_object_ptr tippecanoe = json_hash_get(j, "tippecanoe"); - json_object_ptr id = json_hash_get(j, "id"); + json_object *tippecanoe = json_hash_get(j, "tippecanoe"); + json_object *id = json_hash_get(j, "id"); - json_object_ptr geometries = json_hash_get(geometry, "geometries"); + json_object *geometries = json_hash_get(geometry, "geometries"); if (geometries != nullptr && geometries->type == JSON_ARRAY) { jfa->add_feature(geometries, true, properties, id, tippecanoe, j); } else { diff --git a/geojson-loop.hpp b/geojson-loop.hpp index f1b26584..acdb43d7 100644 --- a/geojson-loop.hpp +++ b/geojson-loop.hpp @@ -4,8 +4,8 @@ struct json_feature_action { std::string fname; - virtual int add_feature(json_object_ptr geometry, bool geometrycollection, json_object_ptr properties, json_object_ptr id, json_object_ptr tippecanoe, json_object_ptr feature) = 0; - virtual void check_crs(json_object_ptr j) = 0; + virtual int add_feature(json_object *geometry, bool geometrycollection, json_object *properties, json_object *id, json_object *tippecanoe, json_object *feature) = 0; + virtual void check_crs(json_object *j) = 0; }; -void parse_json(json_feature_action *action, json_pull_ptr jp); +void parse_json(json_feature_action *action, json_pull_ptr &jp); diff --git a/geojson.cpp b/geojson.cpp index 3a64e08f..8997c5a2 100644 --- a/geojson.cpp +++ b/geojson.cpp @@ -40,8 +40,8 @@ #include "milo/dtoa_milo.h" #include "errors.hpp" -int serialize_geojson_feature(struct serialization_state *sst, json_object_ptr geometry, json_object_ptr properties, json_object_ptr id, int layer, json_object_ptr tippecanoe, json_object_ptr feature, std::string const &layername) { - json_object_ptr geometry_type = json_hash_get(geometry, "type"); +int serialize_geojson_feature(struct serialization_state *sst, json_object *geometry, json_object *properties, json_object *id, int layer, json_object *tippecanoe, json_object *feature, std::string const &layername) { + json_object *geometry_type = json_hash_get(geometry, "type"); if (geometry_type == nullptr) { static int warned = 0; if (!warned) { @@ -59,7 +59,7 @@ int serialize_geojson_feature(struct serialization_state *sst, json_object_ptr g return 0; } - json_object_ptr coordinates = json_hash_get(geometry, "coordinates"); + json_object *coordinates = json_hash_get(geometry, "coordinates"); if (coordinates == nullptr || coordinates->type != JSON_ARRAY) { fprintf(stderr, "%s:%d: feature without coordinates array: ", sst->fname, sst->line); json_context(feature); @@ -83,17 +83,17 @@ int serialize_geojson_feature(struct serialization_state *sst, json_object_ptr g std::string tippecanoe_layername = layername; if (tippecanoe != nullptr) { - json_object_ptr min = json_hash_get(tippecanoe, "minzoom"); + json_object *min = json_hash_get(tippecanoe, "minzoom"); if (min != nullptr && (min->type == JSON_NUMBER)) { tippecanoe_minzoom = integer_zoom(sst->fname, milo::dtoa_milo(min->number())); } - json_object_ptr max = json_hash_get(tippecanoe, "maxzoom"); + json_object *max = json_hash_get(tippecanoe, "maxzoom"); if (max != nullptr && (max->type == JSON_NUMBER)) { tippecanoe_maxzoom = integer_zoom(sst->fname, milo::dtoa_milo(max->number())); } - json_object_ptr ln = json_hash_get(tippecanoe, "layer"); + json_object *ln = json_hash_get(tippecanoe, "layer"); if (ln != nullptr && (ln->type == JSON_STRING)) { tippecanoe_layername = ln->string(); } @@ -186,7 +186,7 @@ int serialize_geojson_feature(struct serialization_state *sst, json_object_ptr g for (const auto &e : entries) { if (e.key->type == JSON_STRING) { - serial_val sv = stringify_value(e.value, sst->fname, sst->line, feature); + serial_val sv = stringify_value(e.value.get(), sst->fname, sst->line, feature); full_keys.emplace_back(key_pool.pool(e.key->string().c_str())); values.push_back(std::move(sv)); @@ -214,12 +214,12 @@ int serialize_geojson_feature(struct serialization_state *sst, json_object_ptr g return serialize_feature(sst, sf, tippecanoe_layername); } -void check_crs(json_object_ptr j, const char *reading) { - json_object_ptr crs = json_hash_get(j, "crs"); +void check_crs(json_object *j, const char *reading) { + json_object *crs = json_hash_get(j, "crs"); if (crs != nullptr) { - json_object_ptr properties = json_hash_get(crs, "properties"); + json_object *properties = json_hash_get(crs, "properties"); if (properties != nullptr) { - json_object_ptr name = json_hash_get(properties, "name"); + json_object *name = json_hash_get(properties, "name"); if (name != nullptr && name->type == JSON_STRING) { if (name->string() != projection->alias) { if (!quiet) { @@ -237,12 +237,12 @@ struct json_serialize_action : json_feature_action { int layer; std::string layername; - int add_feature(json_object_ptr geometry, bool geometrycollection, json_object_ptr properties, json_object_ptr id, json_object_ptr tippecanoe, json_object_ptr feature) { + int add_feature(json_object *geometry, bool geometrycollection, json_object *properties, json_object *id, json_object *tippecanoe, json_object *feature) { sst->line = geometry->parser->line; if (geometrycollection) { int ret = 1; for (size_t g = 0; g < geometry->array().size(); g++) { - ret &= serialize_geojson_feature(sst, geometry->array()[g], properties, id, layer, tippecanoe, feature, layername); + ret &= serialize_geojson_feature(sst, geometry->array()[g].get(), properties, id, layer, tippecanoe, feature, layername); } return ret; } else { @@ -250,12 +250,12 @@ struct json_serialize_action : json_feature_action { } } - void check_crs(json_object_ptr j) { + void check_crs(json_object *j) { ::check_crs(j, fname.c_str()); } }; -void parse_json(struct serialization_state *sst, json_pull_ptr jp, int layer, std::string layername) { +void parse_json(struct serialization_state *sst, json_pull_ptr &jp, int layer, std::string layername) { json_serialize_action jsa; jsa.fname = sst->fname; jsa.sst = sst; diff --git a/geojson.hpp b/geojson.hpp index cb0776a4..8c63318c 100644 --- a/geojson.hpp +++ b/geojson.hpp @@ -24,7 +24,7 @@ struct parse_json_args { json_pull_ptr json_begin_map(char *map, long long len); void json_end_map(json_pull_ptr &jp); -void parse_json(struct serialization_state *sst, json_pull_ptr jp, int layer, std::string layername); +void parse_json(struct serialization_state *sst, json_pull_ptr &jp, int layer, std::string layername); void *run_parse_json(void *v); #endif diff --git a/geometry.hpp b/geometry.hpp index 3aab6c18..454cea61 100644 --- a/geometry.hpp +++ b/geometry.hpp @@ -141,7 +141,7 @@ std::string overzoom(std::vector const &tiles, int nz, int nx, int std::vector const &exclude_prefix, bool do_compress, std::vector> *next_overzoomed_tiles, - bool demultiply, json_object_ptr filter, bool preserve_input_order, + bool demultiply, json_object *filter, bool preserve_input_order, std::unordered_map const &attribute_accum, std::vector const &unidecode_data, double simplification, double tiny_polygon_size, @@ -157,7 +157,7 @@ std::string overzoom(std::vector const &tiles, int nz, int nx, int n std::vector const &exclude_prefix, bool do_compress, std::vector> *next_overzoomed_tiles, - bool demultiply, json_object_ptr filter, bool preserve_input_order, + bool demultiply, json_object *filter, bool preserve_input_order, std::unordered_map const &attribute_accum, std::vector const &unidecode_data, double simplification, double tiny_polygon_size, diff --git a/jsonpull/jsonpull.cpp b/jsonpull/jsonpull.cpp index 6e8a534d..8b23ba75 100644 --- a/jsonpull/jsonpull.cpp +++ b/jsonpull/jsonpull.cpp @@ -89,26 +89,23 @@ static inline int read_wrap(json_pull *j) { // Construct an instance of the right subclass for the given type. // JSON_TRUE / JSON_FALSE / JSON_NULL and the parse-token types are bare // json_objects; the value-bearing types each get their own subclass. +// +// Returns a json_object_ptr (unique_ptr with a type-dispatching deleter, +// see jsonpull.h), so the caller doesn't have to remember which subclass +// was constructed when it eventually deletes. static json_object_ptr make_object(json_type type, json_object *parent, json_pull *jp) { - json_object_ptr o; switch (type) { case JSON_NUMBER: - o = std::make_shared(parent, jp); - break; + return json_object_ptr(new json_number(parent, jp)); case JSON_STRING: - o = std::make_shared(parent, jp); - break; + return json_object_ptr(new json_string(parent, jp)); case JSON_ARRAY: - o = std::make_shared(parent, jp); - break; + return json_object_ptr(new json_array(parent, jp)); case JSON_HASH: - o = std::make_shared(parent, jp); - break; + return json_object_ptr(new json_hash(parent, jp)); default: - o = std::make_shared(type, parent, jp); - break; + return json_object_ptr(new json_object(type, parent, jp)); } - return o; } static json_object_ptr fabricate_object(json_pull *jp, json_object *parent, json_type type) { @@ -119,15 +116,22 @@ static inline json_pull::parse_frame *current_frame(json_pull *j) { return j->container_stack.empty() ? nullptr : &j->container_stack.back(); } -static json_object_ptr add_object(json_pull *j, json_type type) { +// Construct a new node of `type` and install it as a child of the +// current container (or as the parser's root, if the container stack +// is empty). Returns a borrowed pointer into the parser-owned tree; +// the unique_ptr that owns the node lives in whichever vector slot +// we just pushed it into. Returns nullptr on error after setting +// j->error. +static json_object *add_object(json_pull *j, json_type type) { json_pull::parse_frame *f = current_frame(j); - json_object *c = f ? f->container.get() : nullptr; + json_object *c = f ? f->container : nullptr; json_object_ptr o = make_object(type, c, j); + json_object *raw = o.get(); if (f != nullptr) { if (c->type == JSON_ARRAY) { if (f->expect == JSON_ITEM) { - c->array().push_back(o); + c->array().push_back(std::move(o)); f->expect = JSON_COMMA; } else { j->error = "Expected a comma, not a list item"; @@ -135,7 +139,7 @@ static json_object_ptr add_object(json_pull *j, json_type type) { } } else if (c->type == JSON_HASH) { if (f->expect == JSON_VALUE) { - c->entries().back().value = o; + c->entries().back().value = std::move(o); f->expect = JSON_COMMA; } else if (f->expect == JSON_KEY) { if (type != JSON_STRING) { @@ -143,7 +147,7 @@ static json_object_ptr add_object(json_pull *j, json_type type) { return nullptr; } - c->entries().push_back({o, nullptr}); + c->entries().push_back({std::move(o), nullptr}); f->expect = JSON_COLON; } else { j->error = "Expected a comma or colon"; @@ -151,33 +155,34 @@ static json_object_ptr add_object(json_pull *j, json_type type) { } } } else { - // Drop the previous top-level value; replacing the parser's root - // shared_ptr will free it if no one else holds a reference. - j->root = o; + // Replacing the parser's root destroys the previous top-level + // value (if no one called json_disconnect / json_read_tree to + // take ownership of it). + j->root = std::move(o); } - return o; + return raw; } -json_object_ptr json_hash_get(json_object *o, const char *s) { +json_object *json_hash_get(json_object *o, const char *s) { if (o == nullptr || o->type != JSON_HASH) { return nullptr; } for (const auto &e : o->entries()) { if (e.key != nullptr && e.key->type == JSON_STRING && e.key->string() == s) { - return e.value; + return e.value.get(); } } return nullptr; } -json_object_ptr json_hash_get(json_object_ptr o, const char *s) { +json_object *json_hash_get(const json_object_ptr &o, const char *s) { return json_hash_get(o.get(), s); } -json_object_ptr json_read_separators(json_pull_ptr jp, json_separator_callback cb, void *state) { +json_object *json_read_separators(json_pull_ptr &jp, json_separator_callback cb, void *state) { int c; json_pull *j = jp.get(); @@ -226,14 +231,13 @@ again: /////////////////////////// Arrays case '[': { - json_object_ptr o = add_object(j, JSON_ARRAY); + json_object *o = add_object(j, JSON_ARRAY); if (o == nullptr) { return nullptr; } // add_object already installed `o` in the parent (or the - // parser's root); moving the local copy into the frame - // avoids one shared_ptr atomic inc/dec pair per container. - j->container_stack.push_back({std::move(o), JSON_ITEM}); + // parser's root) as a unique_ptr; the frame just borrows. + j->container_stack.push_back({o, JSON_ITEM}); if (cb != nullptr) { cb(JSON_ARRAY, j, state); @@ -249,7 +253,7 @@ again: return nullptr; } - json_object *cc = f->container.get(); + json_object *cc = f->container; if (cc->type != JSON_ARRAY) { j->error = "Found ] not in an array"; return nullptr; @@ -262,23 +266,20 @@ again: } } - // Move the container out of the frame so pop_back doesn't - // drop the last reference; saves one atomic inc/dec. - json_object_ptr ret = std::move(f->container); + // Pop the frame; ownership of `cc` stays with whatever + // surrounding container (or jp->root) installed it. j->container_stack.pop_back(); - return ret; + return cc; } /////////////////////////// Hashes case '{': { - json_object_ptr o = add_object(j, JSON_HASH); + json_object *o = add_object(j, JSON_HASH); if (o == nullptr) { return nullptr; } - // See the [ case above: move into the frame to skip a - // shared_ptr atomic inc/dec round-trip. - j->container_stack.push_back({std::move(o), JSON_KEY}); + j->container_stack.push_back({o, JSON_KEY}); if (cb != nullptr) { cb(JSON_HASH, j, state); @@ -294,7 +295,7 @@ again: return nullptr; } - json_object *cc = f->container.get(); + json_object *cc = f->container; if (cc->type != JSON_HASH) { j->error = "Found } not in a hash"; return nullptr; @@ -307,10 +308,8 @@ again: } } - // See the ] case: move out to skip an atomic refcount round-trip. - json_object_ptr ret = std::move(f->container); j->container_stack.pop_back(); - return ret; + return cc; } /////////////////////////// Null @@ -488,7 +487,7 @@ again: } } - json_object_ptr n = add_object(j, JSON_NUMBER); + json_object *n = add_object(j, JSON_NUMBER); if (n != nullptr) { double d = atof(j->number_buffer.c_str()); n->set_number(d); @@ -642,7 +641,7 @@ again: return nullptr; } - json_object_ptr s = add_object(j, JSON_STRING); + json_object *s = add_object(j, JSON_STRING); if (s != nullptr) { // Copy (don't move) so j->string_buffer retains its // grown capacity for the next token. The copy is a @@ -659,48 +658,71 @@ again: return nullptr; } -json_object_ptr json_read(json_pull_ptr j) { +json_object *json_read(json_pull_ptr &j) { return json_read_separators(j, nullptr, nullptr); } -json_object_ptr json_read_tree(json_pull_ptr p) { - json_object_ptr j; +// Forward declaration so json_read_tree can clear back-pointers on +// the tree it hands out -- this lets callers (like the filter loaders) +// keep the returned tree past the parser's lifetime without having to +// follow up with a separate json_disconnect call. +static void clear_back_pointers(json_object *o); + +json_object_ptr json_read_tree(json_pull_ptr &p) { + json_object *j; while ((j = json_read(p)) != nullptr) { if (j->parent == nullptr) { - return j; + // The parser owns the top-level value via p->root; + // transfer ownership out to the caller and detach + // the subtree from the parser so the caller can + // outlive the json_pull. + json_object_ptr tree = std::move(p->root); + clear_back_pointers(tree.get()); + return tree; } } return nullptr; } -// Splice `o` out of its parent's array or object, dropping the -// parent's owning reference. After this returns, the parent no longer -// holds any pointer to `o`; the caller's reference is the only thing -// keeping the subtree alive. +// Take ownership of `o` away from its parent (or from the parser's +// root) by moving the owning json_object_ptr out of whatever vector +// slot or hash entry holds it. Returns the unique_ptr to the caller, +// who is now solely responsible for it. Returns an empty +// json_object_ptr if `o` is not currently owned by a parent or by +// the parser (e.g. already detached, or only borrowed from somewhere +// untracked). // // For a hash, removing a single key or value individually would -// disturb the surrounding key/value pairing, so we replace the removed -// half with a placeholder JSON_NULL and only erase the entry once both -// halves have been detached. This matches the historical -// json_disconnect semantics. -static void splice_from_parent(json_object *o) { +// disturb the surrounding key/value pairing, so we replace the +// extracted half with a fresh JSON_NULL placeholder and only erase +// the entry once both halves have been detached. This matches the +// historical json_disconnect semantics for partially-disconnected +// pairs. +static json_object_ptr take_from_owner(json_object *o) { if (o == nullptr) { - return; + return nullptr; } json_object *parent = o->parent; if (parent == nullptr) { - return; + // Top-level value: the parser owns it via root, unless the + // caller already moved it out. + json_pull *parser = o->parser; + if (parser != nullptr && parser->root.get() == o) { + return std::move(parser->root); + } + return nullptr; } if (parent->type == JSON_ARRAY) { auto &arr = parent->array(); for (size_t i = 0; i < arr.size(); i++) { if (arr[i].get() == o) { + json_object_ptr taken = std::move(arr[i]); arr.erase(arr.begin() + i); - break; + return taken; } } } else if (parent->type == JSON_HASH) { @@ -708,49 +730,43 @@ static void splice_from_parent(json_object *o) { for (size_t i = 0; i < entries.size(); i++) { auto &e = entries[i]; if (e.key.get() == o) { + json_object_ptr taken = std::move(e.key); e.key = fabricate_object(parent->parser, parent, JSON_NULL); if (e.value != nullptr && e.value->type == JSON_NULL && e.key->type == JSON_NULL) { entries.erase(entries.begin() + i); } - break; + return taken; } if (e.value.get() == o) { + json_object_ptr taken = std::move(e.value); e.value = fabricate_object(parent->parser, parent, JSON_NULL); if (e.key != nullptr && e.key->type == JSON_NULL && e.value->type == JSON_NULL) { entries.erase(entries.begin() + i); } - break; + return taken; } } } + + return nullptr; } -// json_free splices `o` out of its parent (if any) so that the -// parent no longer keeps the subtree alive, then drops the caller's -// reference. The subtree is freed when the last reference is gone -// (typically right here, since the parent's reference was just -// dropped). geojson-loop.cpp relies on this to release each feature -// after it has been serialized, so that already-serialized features -// don't sit in memory while subsequent features are parsed. +// json_free splices `o` out of its parent (if any), or out of the +// parser's root (if `o` is the most recently completed top-level +// value), and destroys the subtree. After this call, `o` is a +// dangling pointer and must not be used. // -// If `o` is the parser's current root (the most recently completed -// top-level value), drop the parser's reference too -- otherwise a -// line-delimited stream would always hold the previously-completed -// feature until the next one started parsing. +// geojson-loop.cpp relies on this to release each feature after it +// has been serialized, so that already-serialized features don't sit +// in memory while subsequent features are parsed. // // Unlike json_disconnect, this does NOT walk the subtree clearing // parent/parser back-pointers, because the subtree is about to be -// destroyed and those pointers will never be observed again. -void json_free(json_object_ptr &o) { - if (o != nullptr) { - splice_from_parent(o.get()); - - json_pull *parser = o->parser; - if (parser != nullptr && parser->root.get() == o.get()) { - parser->root.reset(); - } - } - o.reset(); +// destroyed and those pointers will never be observed again -- the +// unique_ptr returned by take_from_owner goes out of scope at the end +// of this function and runs the type-dispatching deleter. +void json_free(json_object *o) { + (void) take_from_owner(o); } // Walk the subtree clearing parent/parser back-pointers so the detached @@ -776,23 +792,12 @@ static void clear_back_pointers(json_object *o) { o->parser = nullptr; } -void json_disconnect(json_object_ptr o) { - if (o == nullptr) { - return; +json_object_ptr json_disconnect(json_object *o) { + json_object_ptr taken = take_from_owner(o); + if (taken != nullptr) { + clear_back_pointers(taken.get()); } - - // Splice o out of its parent's array or object. The parent's vector - // holds the shared_ptr to this child; erasing it removes one reference, - // but the caller still holds `o`, so the subtree stays alive. - splice_from_parent(o.get()); - - // Drop the parser's reference to this subtree if it was the root. - json_pull *parser = o->parser; - if (parser != nullptr && parser->root.get() == o.get()) { - parser->root.reset(); - } - - clear_back_pointers(o.get()); + return taken; } static void string_append_c(std::string &val, char c) { @@ -803,7 +808,7 @@ static void string_append(std::string &val, const char *add) { val.append(add); } -static void json_print_one(std::string &val, json_object *o) { +static void json_print_one(std::string &val, const json_object *o) { if (o == nullptr) { string_append(val, "..."); } else if (o->type == JSON_STRING) { @@ -852,7 +857,7 @@ static void json_print_one(std::string &val, json_object *o) { } } -static void json_print(std::string &val, json_object *o) { +static void json_print(std::string &val, const json_object *o) { if (o == nullptr) { // Hash value in incompletely read hash string_append(val, "..."); @@ -884,8 +889,8 @@ static void json_print(std::string &val, json_object *o) { } } -std::string json_stringify(json_object_ptr o) { +std::string json_stringify(const json_object *o) { std::string val; - json_print(val, o.get()); + json_print(val, o); return val; } diff --git a/jsonpull/jsonpull.h b/jsonpull/jsonpull.h index a6d5cc6c..170a8d21 100644 --- a/jsonpull/jsonpull.h +++ b/jsonpull/jsonpull.h @@ -31,7 +31,31 @@ typedef enum json_type { struct json_object; struct json_pull; -typedef std::shared_ptr json_object_ptr; +// json_object is non-virtual so that JSON_TRUE / JSON_FALSE / JSON_NULL +// nodes don't have to pay for a vptr, but the typed subclasses +// (json_number, json_string, json_array, json_hash) have non-trivial +// destructors that need to run to free their std::vector / std::string +// members. So json_object_ptr is given a custom empty deleter that +// dispatches on `type` and static_casts to the right subclass before +// `delete`. The deleter is stateless, so the unique_ptr stays one +// pointer wide. +struct json_object_deleter { + void operator()(json_object *p) const noexcept; +}; + +// Ownership of a JSON subtree is unique: every node has a single owner, +// which is either its parent (via a json_object_ptr in the parent's +// vector or hash entry) or, for the root, the parser (via jp->root) or +// the caller (after json_read_tree / json_disconnect). +// +// Callers receive borrowed `json_object *` views from json_read, +// json_hash_get, etc.; those pointers stay valid as long as the owning +// container is intact (which, for json_read results, means "until the +// next json_read, json_free, or json_disconnect call on that subtree"). +// +// json_pull_ptr stays a shared_ptr because the parser is created once +// and freed once and the cost of shared_ptr there is irrelevant. +typedef std::unique_ptr json_object_ptr; typedef std::shared_ptr json_pull_ptr; // A single key/value pair inside a JSON_HASH. The pairs are stored in @@ -60,12 +84,12 @@ struct json_entry { // outlive the original parser. // // json_object intentionally has no virtual functions and no virtual -// destructor: subclasses are constructed via std::make_shared(), -// and std::shared_ptr remembers the deleter from the original type, so -// destroying a shared_ptr that actually points at a -// json_string still runs ~json_string(). Dispatch on `type` is what the -// rest of the code already does. The accessor methods assert at debug -// time that the type matches before downcasting. +// destructor; the json_object_ptr deleter (see below in this header) +// switches on `type` and static_casts to the correct subclass before +// `delete`, so each subclass's destructor still runs without costing +// a vptr per node. Dispatch on `type` is what the rest of the code +// already does. The accessor methods assert at debug time that the +// type matches before downcasting. struct json_object { json_object *parent = nullptr; @@ -218,6 +242,35 @@ inline const std::vector &json_object::entries() const { return static_cast(this)->entries_value; } +inline void json_object_deleter::operator()(json_object *p) const noexcept { + if (p == nullptr) { + return; + } + // Dispatch on the discriminator so the correct subclass destructor + // runs. json_object has no virtual destructor, so a bare `delete p` + // would skip the std::vector / std::string members of the subclass. + switch (p->type) { + case JSON_NUMBER: + delete static_cast(p); + break; + case JSON_STRING: + delete static_cast(p); + break; + case JSON_ARRAY: + delete static_cast(p); + break; + case JSON_HASH: + delete static_cast(p); + break; + default: + // JSON_TRUE / JSON_FALSE / JSON_NULL (and the parse-token + // types, which never appear as owned nodes) are bare + // json_objects with no extra fields. + delete p; + break; + } +} + struct json_pull { const char *error = nullptr; // points at a string literal; no allocation int line = 1; @@ -228,18 +281,25 @@ struct json_pull { ssize_t buffer_tail = 0; ssize_t buffer_head = 0; - // Stack of currently-open containers; the top is the innermost container - // being parsed. Each frame also remembers what token is expected next - // (an item, a comma, a key, a colon, or a value). This stack is the - // only place the parser-only `expect` state lives, so it does not - // pollute json_object once parsing finishes. Replaces the previous - // single `container` pointer / parent walk, which previously required - // enable_shared_from_this on every json_object instance. + // Stack of currently-open containers; the top is the innermost + // container being parsed. Each frame also remembers what token is + // expected next (an item, a comma, a key, a colon, or a value). + // The frame's `container` is a borrowed raw pointer; actual + // ownership of the in-progress container lives in either the + // surrounding container's vector (for nested containers) or + // `root` (for the outermost container). struct parse_frame { - json_object_ptr container; + json_object *container; json_type expect; }; std::vector container_stack; + + // The most recently completed top-level value. The parser owns + // it (as a unique_ptr) until either: the next top-level value + // starts parsing (the old root is destroyed), the caller calls + // json_read_tree (ownership is transferred out), or the caller + // calls json_free / json_disconnect (the parser's reference is + // dropped explicitly). json_object_ptr root; // Scratch buffers reused across tokens so we don't reallocate per @@ -257,30 +317,54 @@ json_pull_ptr json_begin_string(const char *s); json_pull_ptr json_begin(ssize_t (*read)(struct json_pull *, char *buffer, size_t n), void *source); -// json_end is now a thin convenience that resets the caller's json_pull_ptr. -// The parser (and any tree it still owns) is freed when the last shared_ptr -// to it is dropped, so calling json_end is optional if the json_pull_ptr will -// go out of scope on its own. +// json_end is a thin convenience that resets the caller's json_pull_ptr. +// The parser (and any tree it still owns) is freed when the last +// shared_ptr to it is dropped, so calling json_end is optional if the +// json_pull_ptr will go out of scope on its own. void json_end(json_pull_ptr &p); typedef void (*json_separator_callback)(json_type type, json_pull *j, void *state); -json_object_ptr json_read_tree(json_pull_ptr j); -json_object_ptr json_read(json_pull_ptr j); -json_object_ptr json_read_separators(json_pull_ptr j, json_separator_callback cb, void *state); +// json_read returns a borrowed pointer to the next completed JSON node +// in the stream. The returned pointer is valid until the next call that +// extends or trims the parser's tree (the next json_read on the same +// parser, a json_free on the same node, or a json_disconnect that +// extracts the node). Returns nullptr at end of input or on error. +// +// For top-level values, ownership stays with the parser (via jp->root); +// for nested values, ownership stays with the enclosing container. +json_object *json_read(json_pull_ptr &j); +json_object *json_read_separators(json_pull_ptr &j, json_separator_callback cb, void *state); -// json_free now just resets the caller's json_object_ptr. The subtree is -// destroyed when the last shared_ptr to it is dropped (typically by also -// being removed from its parent or parser). -void json_free(json_object_ptr &j); +// json_read_tree drains the next top-level value out of the parser +// and hands ownership to the caller. After it returns, jp->root is +// empty, the parent/parser back-pointers throughout the subtree have +// been cleared, and the caller's json_object_ptr is the only thing +// keeping the tree alive. The returned tree can outlive the +// json_pull it was parsed from. +json_object_ptr json_read_tree(json_pull_ptr &j); -// Splice o out of its parent's array/object and clear parent/parser back-pointers -// throughout the detached subtree so it can outlive the original parser. -void json_disconnect(json_object_ptr j); +// json_free splices `o` out of its parent (if any), or clears the +// parser's root if `o` is the parser's current top-level value, and +// destroys the subtree. After this call, `o` is a dangling pointer +// that must not be used. Safe to call with nullptr. +void json_free(json_object *o); -json_object_ptr json_hash_get(json_object_ptr o, const char *s); -json_object_ptr json_hash_get(json_object *o, const char *s); +// Splice `o` out of its parent's array/object (or out of the parser's +// root), walk the detached subtree clearing parent/parser back-pointers, +// and return ownership of the subtree to the caller as a +// json_object_ptr. After this returns, the parser no longer references +// any node in the subtree, and the subtree can outlive the original +// parser. +json_object_ptr json_disconnect(json_object *o); -std::string json_stringify(json_object_ptr o); +// Look up `s` in the hash `o`. Returns a borrowed pointer; ownership +// stays with the hash. nullptr if `o` is not a hash, or `s` is absent, +// or the matching value is null. Accepts a json_object_ptr by reference +// as a convenience so callers don't have to write `.get()`. +json_object *json_hash_get(const json_object_ptr &o, const char *s); +json_object *json_hash_get(json_object *o, const char *s); + +std::string json_stringify(const json_object *o); #endif diff --git a/jsontool.cpp b/jsontool.cpp index 52cb1c33..014b1f6e 100644 --- a/jsontool.cpp +++ b/jsontool.cpp @@ -140,12 +140,12 @@ std::string sort_quote(const char *s) { return ret; } -void out(std::string const &s, int type, json_object_ptr properties) { +void out(std::string const &s, int type, json_object *properties) { if (extract != NULL) { std::string extracted = sort_quote("null"); bool found = false; - json_object_ptr o = json_hash_get(properties, extract); + json_object *o = json_hash_get(properties, extract); if (o != nullptr) { found = true; if (o->type == JSON_STRING) { @@ -204,7 +204,7 @@ void out(std::string const &s, int type, json_object_ptr properties) { std::string prev_joinkey; -void join_csv(json_object_ptr j) { +void join_csv(json_object *j) { if (header.size() == 0) { std::string s = csv_getline(csvfile); if (s.size() == 0) { @@ -230,8 +230,8 @@ void join_csv(json_object_ptr j) { } } - json_object_ptr properties = json_hash_get(j, "properties"); - json_object_ptr key; + json_object *properties = json_hash_get(j, "properties"); + json_object *key = nullptr; if (properties != nullptr) { key = json_hash_get(properties, header[0].c_str()); @@ -320,30 +320,28 @@ void join_csv(json_object_ptr j) { } if (attr_type != JSON_NULL) { - auto ko = std::make_shared(properties.get(), properties->parser); - ko->string_value = k; + json_object_ptr ko(new json_string(properties, properties->parser)); + ko->string() = k; json_object_ptr vo; if (attr_type == JSON_STRING) { - auto s = std::make_shared(properties.get(), properties->parser); - s->string_value = v; - vo = s; + vo = json_object_ptr(new json_string(properties, properties->parser)); + vo->string() = v; } else if (attr_type == JSON_NUMBER) { - auto n = std::make_shared(properties.get(), properties->parser); - n->set_number(atof(v.c_str())); - vo = n; + vo = json_object_ptr(new json_number(properties, properties->parser)); + vo->set_number(atof(v.c_str())); } else { abort(); } - properties->entries().push_back({ko, vo}); + properties->entries().push_back({std::move(ko), std::move(vo)}); } } } } struct json_join_action : json_feature_action { - int add_feature(json_object_ptr geometry, bool, json_object_ptr, json_object_ptr, json_object_ptr, json_object_ptr feature) { + int add_feature(json_object *geometry, bool, json_object *, json_object *, json_object *, json_object *feature) { if (feature != geometry) { // a real feature, not a bare geometry if (csvfile != NULL) { join_csv(feature); @@ -357,7 +355,7 @@ struct json_join_action : json_feature_action { return 1; } - void check_crs(json_object_ptr) { + void check_crs(json_object *) { } }; diff --git a/main.cpp b/main.cpp index 95a4096b..c11c3456 100644 --- a/main.cpp +++ b/main.cpp @@ -1215,7 +1215,7 @@ double round_droprate(double r) { return std::round(r * 100000.0) / 100000.0; } -std::pair read_input(std::vector &sources, char *fname, int maxzoom, int minzoom, int basezoom, double basezoom_marker_width, sqlite3 *outdb, const char *outdir, std::set *exclude, std::set *include, int exclude_all, json_object_ptr filter, double droprate, int buffer, const char *tmpdir, double gamma, int read_parallel, int forcetable, const char *attribution, bool uses_gamma, long long *file_bbox, long long *file_bbox1, long long *file_bbox2, const char *prefilter, const char *postfilter, const char *description, bool guess_maxzoom, bool guess_cluster_maxzoom, std::unordered_map const *attribute_types, const char *pgm, std::unordered_map const *attribute_accum, std::map const &attribute_descriptions, std::string const &commandline, int minimum_maxzoom) { +std::pair read_input(std::vector &sources, char *fname, int maxzoom, int minzoom, int basezoom, double basezoom_marker_width, sqlite3 *outdb, const char *outdir, std::set *exclude, std::set *include, int exclude_all, json_object *filter, double droprate, int buffer, const char *tmpdir, double gamma, int read_parallel, int forcetable, const char *attribution, bool uses_gamma, long long *file_bbox, long long *file_bbox1, long long *file_bbox2, const char *prefilter, const char *postfilter, const char *description, bool guess_maxzoom, bool guess_cluster_maxzoom, std::unordered_map const *attribute_types, const char *pgm, std::unordered_map const *attribute_accum, std::map const &attribute_descriptions, std::string const &commandline, int minimum_maxzoom) { int ret = EXIT_SUCCESS; std::vector readers; @@ -2892,7 +2892,7 @@ void set_attribute_value(const char *arg) { exit(EXIT_JSON); } - serial_val val = stringify_value(e.value, "json", 1, o); + serial_val val = stringify_value(e.value.get(), "json", 1, o.get()); set_attributes.emplace(e.key->string(), val); i++; } @@ -2934,7 +2934,7 @@ void parse_json_source(const char *arg, struct source &src) { exit(EXIT_JSON); } - json_object_ptr fname = json_hash_get(o, "file"); + json_object *fname = json_hash_get(o, "file"); if (fname == nullptr || fname->type != JSON_STRING) { fprintf(stderr, "%s: -L%s: requires \"file\": filename\n", *av, arg); exit(EXIT_JSON); @@ -2942,17 +2942,17 @@ void parse_json_source(const char *arg, struct source &src) { src.file = fname->string(); - json_object_ptr layer = json_hash_get(o, "layer"); + json_object *layer = json_hash_get(o, "layer"); if (layer != nullptr && layer->type == JSON_STRING) { src.layer = layer->string(); } - json_object_ptr description = json_hash_get(o, "description"); + json_object *description = json_hash_get(o, "description"); if (description != nullptr && description->type == JSON_STRING) { src.description = description->string(); } - json_object_ptr format = json_hash_get(o, "format"); + json_object *format = json_hash_get(o, "format"); if (format != nullptr && format->type == JSON_STRING) { src.format = format->string(); } @@ -3846,7 +3846,7 @@ int main(int argc, char **argv) { auto input_ret = read_input(sources, name ? name : out_mbtiles ? out_mbtiles : out_dir, - maxzoom, minzoom, basezoom, basezoom_marker_width, outdb, out_dir, &exclude, &include, exclude_all, filter, droprate, buffer, tmpdir, gamma, read_parallel, forcetable, attribution, gamma != 0, file_bbox, file_bbox1, file_bbox2, prefilter, postfilter, description, guess_maxzoom, guess_cluster_maxzoom, &attribute_types, argv[0], &attribute_accum, attribute_descriptions, commandline, minimum_maxzoom); + maxzoom, minzoom, basezoom, basezoom_marker_width, outdb, out_dir, &exclude, &include, exclude_all, filter.get(), droprate, buffer, tmpdir, gamma, read_parallel, forcetable, attribution, gamma != 0, file_bbox, file_bbox1, file_bbox2, prefilter, postfilter, description, guess_maxzoom, guess_cluster_maxzoom, &attribute_types, argv[0], &attribute_accum, attribute_descriptions, commandline, minimum_maxzoom); ret = std::get<0>(input_ret); diff --git a/overzoom.cpp b/overzoom.cpp index b0b16910..05dce3e7 100644 --- a/overzoom.cpp +++ b/overzoom.cpp @@ -264,7 +264,7 @@ int main(int argc, char **argv) { its.push_back(std::move(t)); } - out = overzoom(its, nz, nx, ny, detail, buffer, keep, exclude, exclude_prefix, do_compress, NULL, demultiply, json_filter, preserve_input_order, attribute_accum, unidecode_data, simplification, tiny_polygon_size, std::vector(), "", "", SIZE_MAX, std::vector(), deduplicate_by_id); + out = overzoom(its, nz, nx, ny, detail, buffer, keep, exclude, exclude_prefix, do_compress, NULL, demultiply, json_filter.get(), preserve_input_order, attribute_accum, unidecode_data, simplification, tiny_polygon_size, std::vector(), "", "", SIZE_MAX, std::vector(), deduplicate_by_id); } FILE *f = fopen(outfile, "wb"); diff --git a/plugin.cpp b/plugin.cpp index b9d5f2de..7e07dea3 100644 --- a/plugin.cpp +++ b/plugin.cpp @@ -143,16 +143,23 @@ std::vector parse_layers(int fd, int z, unsigned x, unsigned y, std:: } // Reads from the prefilter -serial_feature parse_feature(json_pull_ptr jp, int z, unsigned x, unsigned y, std::vector> *layermaps, size_t tiling_seg, std::vector> *layer_unmaps, bool postfilter, key_pool &key_pool) { +serial_feature parse_feature(json_pull_ptr &jp, int z, unsigned x, unsigned y, std::vector> *layermaps, size_t tiling_seg, std::vector> *layer_unmaps, bool postfilter, key_pool &key_pool) { serial_feature sf; while (1) { - json_object_ptr j = json_read(jp); + // json_read returns each token as the parser produces it, including + // intermediate (still incomplete) container nodes. We must NOT free + // these intermediates here: they belong to the larger feature hash + // still being assembled, and freeing them would splice them out of + // the parent and corrupt the in-progress tree. We only free `j` + // after we have successfully processed a complete Feature hash + // (just before returning), or `jp->root` when the stream ends. + json_object *j = json_read(jp); if (j == nullptr) { if (jp->error != nullptr) { fprintf(stderr, "Filter output:%d: %s: ", jp->line, jp->error); if (jp->root != nullptr) { - json_context(jp->root); + json_context(jp->root.get()); } else { fprintf(stderr, "\n"); } @@ -164,7 +171,7 @@ serial_feature parse_feature(json_pull_ptr jp, int z, unsigned x, unsigned y, st return sf; } - json_object_ptr type = json_hash_get(j, "type"); + json_object *type = json_hash_get(j, "type"); if (type == nullptr || type->type != JSON_STRING) { continue; } @@ -172,21 +179,21 @@ serial_feature parse_feature(json_pull_ptr jp, int z, unsigned x, unsigned y, st continue; } - json_object_ptr geometry = json_hash_get(j, "geometry"); + json_object *geometry = json_hash_get(j, "geometry"); if (geometry == nullptr) { fprintf(stderr, "Filter output:%d: filtered feature with no geometry: ", jp->line); json_context(j); exit(EXIT_JSON); } - json_object_ptr properties = json_hash_get(j, "properties"); + json_object *properties = json_hash_get(j, "properties"); if (properties == nullptr || (properties->type != JSON_HASH && properties->type != JSON_NULL)) { fprintf(stderr, "Filter output:%d: feature without properties hash: ", jp->line); json_context(j); exit(EXIT_JSON); } - json_object_ptr geometry_type = json_hash_get(geometry, "type"); + json_object *geometry_type = json_hash_get(geometry, "type"); if (geometry_type == nullptr) { fprintf(stderr, "Filter output:%d: null geometry (additional not reported): ", jp->line); json_context(j); @@ -199,7 +206,7 @@ serial_feature parse_feature(json_pull_ptr jp, int z, unsigned x, unsigned y, st exit(EXIT_JSON); } - json_object_ptr coordinates = json_hash_get(geometry, "coordinates"); + json_object *coordinates = json_hash_get(geometry, "coordinates"); if (coordinates == nullptr || coordinates->type != JSON_ARRAY) { fprintf(stderr, "Filter output:%d: feature without coordinates array: ", jp->line); json_context(j); @@ -248,29 +255,29 @@ serial_feature parse_feature(json_pull_ptr jp, int z, unsigned x, unsigned y, st sf.has_id = false; std::string layername = "unknown"; - json_object_ptr tippecanoe = json_hash_get(j, "tippecanoe"); + json_object *tippecanoe = json_hash_get(j, "tippecanoe"); if (tippecanoe != nullptr) { - json_object_ptr layer = json_hash_get(tippecanoe, "layer"); + json_object *layer = json_hash_get(tippecanoe, "layer"); if (layer != nullptr && layer->type == JSON_STRING) { layername = layer->string(); } - json_object_ptr index = json_hash_get(tippecanoe, "index"); + json_object *index = json_hash_get(tippecanoe, "index"); if (index != nullptr && index->type == JSON_NUMBER) { sf.index = index->number(); } - json_object_ptr sequence = json_hash_get(tippecanoe, "sequence"); + json_object *sequence = json_hash_get(tippecanoe, "sequence"); if (sequence != nullptr && sequence->type == JSON_NUMBER) { sf.seq = sequence->number(); } - json_object_ptr extent = json_hash_get(tippecanoe, "extent"); + json_object *extent = json_hash_get(tippecanoe, "extent"); if (extent != nullptr && extent->type == JSON_NUMBER) { sf.extent = extent->number(); } - json_object_ptr dropped = json_hash_get(tippecanoe, "dropped"); + json_object *dropped = json_hash_get(tippecanoe, "dropped"); if (dropped != nullptr && dropped->type == JSON_TRUE) { sf.dropped = FEATURE_DROPPED; // dropped } else { @@ -295,7 +302,7 @@ serial_feature parse_feature(json_pull_ptr jp, int z, unsigned x, unsigned y, st } } - json_object_ptr id = json_hash_get(j, "id"); + json_object *id = json_hash_get(j, "id"); if (id != nullptr && id->type == JSON_NUMBER) { sf.id = id->number(); if (id->large_unsigned() > 0) { @@ -345,7 +352,7 @@ serial_feature parse_feature(json_pull_ptr jp, int z, unsigned x, unsigned y, st if (properties->type == JSON_HASH) { for (const auto &e : properties->entries()) { - serial_val v = stringify_value(e.value, "Filter output", jp->line, j); + serial_val v = stringify_value(e.value.get(), "Filter output", jp->line, j); // Nulls can be excluded here because the expression evaluation filter // would have already run before prefiltering @@ -361,8 +368,11 @@ serial_feature parse_feature(json_pull_ptr jp, int z, unsigned x, unsigned y, st } } + json_free(j); return sf; } + + json_free(j); } } diff --git a/plugin.hpp b/plugin.hpp index 2a1feab6..5ad99092 100644 --- a/plugin.hpp +++ b/plugin.hpp @@ -1,4 +1,4 @@ struct key_pool; std::vector filter_layers(const char *filter, std::vector &layer, unsigned z, unsigned x, unsigned y, std::vector> *layermaps, size_t tiling_seg, std::vector> *layer_unmaps, int extent); void setup_filter(const char *filter, int *write_to, int *read_from, pid_t *pid, unsigned z, unsigned x, unsigned y); -serial_feature parse_feature(json_pull_ptr jp, int z, unsigned x, unsigned y, std::vector> *layermaps, size_t tiling_seg, std::vector> *layer_unmaps, bool filters, key_pool &key_pool); +serial_feature parse_feature(json_pull_ptr &jp, int z, unsigned x, unsigned y, std::vector> *layermaps, size_t tiling_seg, std::vector> *layer_unmaps, bool filters, key_pool &key_pool); diff --git a/pmtiles_file.cpp b/pmtiles_file.cpp index 9a8c62a1..57695b1a 100644 --- a/pmtiles_file.cpp +++ b/pmtiles_file.cpp @@ -422,21 +422,21 @@ sqlite3 *pmtilesmeta2tmp(const char *fname, const char *pmtiles_map) { state.nospace = true; state.json_write_string("vector_layers"); state.nospace = true; - state.json_write_json(json_stringify(e.value)); + state.json_write_json(json_stringify(e.value.get())); } else if (key == "tilestats" && e.value->type == JSON_HASH) { has_json = true; state.nospace = true; state.json_write_string("tilestats"); state.nospace = true; - state.json_write_json(json_stringify(e.value)); + state.json_write_json(json_stringify(e.value.get())); } else if (key == "strategies" && e.value->type == JSON_ARRAY) { - sql = sqlite3_mprintf("INSERT INTO metadata (name, value) VALUES ('strategies', %Q);", json_stringify(e.value).c_str()); + sql = sqlite3_mprintf("INSERT INTO metadata (name, value) VALUES ('strategies', %Q);", json_stringify(e.value.get()).c_str()); if (sqlite3_exec(db, sql, NULL, NULL, &err) != SQLITE_OK) { fprintf(stderr, "set %s in metadata: %s\n", key.c_str(), err); } sqlite3_free(sql); } else if (key == "tippecanoe_decisions" && e.value->type == JSON_HASH) { - sql = sqlite3_mprintf("INSERT INTO metadata (name, value) VALUES ('tippecanoe_decisions', %Q);", json_stringify(e.value).c_str()); + sql = sqlite3_mprintf("INSERT INTO metadata (name, value) VALUES ('tippecanoe_decisions', %Q);", json_stringify(e.value.get()).c_str()); if (sqlite3_exec(db, sql, NULL, NULL, &err) != SQLITE_OK) { fprintf(stderr, "set %s in metadata: %s\n", key.c_str(), err); } diff --git a/read_json.cpp b/read_json.cpp index 45a533f4..13798ee6 100644 --- a/read_json.cpp +++ b/read_json.cpp @@ -42,7 +42,7 @@ int mb_geometry[GEOM_TYPES] = { VT_POLYGON, }; -void json_context(json_object_ptr j) { +void json_context(json_object *j) { std::string s = json_stringify(j); if (s.size() >= 500) { @@ -53,7 +53,7 @@ void json_context(json_object_ptr j) { fprintf(stderr, "in JSON object %s\n", s.c_str()); } -void parse_coordinates(int t, json_object_ptr j, drawvec &out, int op, const char *fname, int line, json_object_ptr feature) { +void parse_coordinates(int t, json_object *j, drawvec &out, int op, const char *fname, int line, json_object *feature) { if (j == nullptr || j->type != JSON_ARRAY) { fprintf(stderr, "%s:%d: expected array for geometry type %d: ", fname, line, t); json_context(feature); @@ -72,7 +72,7 @@ void parse_coordinates(int t, json_object_ptr j, drawvec &out, int op, const cha } } - parse_coordinates(within, j->array()[i], out, op, fname, line, feature); + parse_coordinates(within, j->array()[i].get(), out, op, fname, line, feature); } } else { if (j->array().size() >= 2 && j->array()[0]->type == JSON_NUMBER && j->array()[1]->type == JSON_NUMBER) { @@ -121,7 +121,7 @@ void parse_coordinates(int t, json_object_ptr j, drawvec &out, int op, const cha // type and stringified value. All numeric values, even if they are integers, // even integers that are too large to fit in a double but will still be // stringified with their original precision, are recorded here as mvt_double. -serial_val stringify_value(json_object_ptr value, const char *reading, int line, json_object_ptr feature) { +serial_val stringify_value(json_object *value, const char *reading, int line, json_object *feature) { serial_val sv; if (value != nullptr) { @@ -176,9 +176,9 @@ static std::vector to_feature(drawvec &geom) { return out; } -std::pair parse_geometry(json_object_ptr geometry, json_pull_ptr jp, json_object_ptr j, +std::pair parse_geometry(json_object *geometry, json_pull_ptr &jp, json_object *j, int z, int x, int y, long long extent, bool fix_longitudes, bool mvt_style) { - json_object_ptr geometry_type = json_hash_get(geometry, "type"); + json_object *geometry_type = json_hash_get(geometry, "type"); if (geometry_type == nullptr) { fprintf(stderr, "Filter output:%d: null geometry (additional not reported): ", jp->line); json_context(j); @@ -191,7 +191,7 @@ std::pair parse_geometry(json_object_ptr geometry, json_pull_ptr j exit(EXIT_JSON); } - json_object_ptr coordinates = json_hash_get(geometry, "coordinates"); + json_object *coordinates = json_hash_get(geometry, "coordinates"); if (coordinates == nullptr || coordinates->type != JSON_ARRAY) { fprintf(stderr, "Filter output:%d: geometry without coordinates array: ", jp->line); json_context(j); @@ -305,12 +305,12 @@ std::vector parse_layers(FILE *fp, int z, unsigned x, unsigned y, int json_pull_ptr jp = json_begin_file(fp); while (1) { - json_object_ptr j = json_read(jp); + json_object *j = json_read(jp); if (j == nullptr) { if (jp->error != nullptr) { fprintf(stderr, "Filter output:%d: %s: ", jp->line, jp->error); if (jp->root != nullptr) { - json_context(jp->root); + json_context(jp->root.get()); } else { fprintf(stderr, "\n"); } @@ -321,7 +321,12 @@ std::vector parse_layers(FILE *fp, int z, unsigned x, unsigned y, int break; } - json_object_ptr type = json_hash_get(j, "type"); + // json_read returns each parser token in sequence, including + // intermediate (still-incomplete) container nodes. Freeing those + // here would splice them out of the feature hash being built + // up, so only free `j` once we have processed a complete + // Feature (or `jp->root` when the stream ends). + json_object *type = json_hash_get(j, "type"); if (type == nullptr || type->type != JSON_STRING) { continue; } @@ -329,7 +334,7 @@ std::vector parse_layers(FILE *fp, int z, unsigned x, unsigned y, int continue; } - json_object_ptr properties = json_hash_get(j, "properties"); + json_object *properties = json_hash_get(j, "properties"); if (properties == nullptr || (properties->type != JSON_HASH && properties->type != JSON_NULL)) { fprintf(stderr, "Filter output:%d: feature without properties hash: ", jp->line); json_context(j); @@ -337,8 +342,8 @@ std::vector parse_layers(FILE *fp, int z, unsigned x, unsigned y, int } std::string layername = "unknown"; - json_object_ptr tippecanoe = json_hash_get(j, "tippecanoe"); - json_object_ptr layer; + json_object *tippecanoe = json_hash_get(j, "tippecanoe"); + json_object *layer = nullptr; if (tippecanoe != nullptr) { layer = json_hash_get(tippecanoe, "layer"); if (layer != nullptr && layer->type == JSON_STRING) { @@ -356,7 +361,7 @@ std::vector parse_layers(FILE *fp, int z, unsigned x, unsigned y, int } auto l = ret.find(layername); - json_object_ptr geometry = json_hash_get(j, "geometry"); + json_object *geometry = json_hash_get(j, "geometry"); if (geometry == nullptr) { fprintf(stderr, "Filter output:%d: filtered feature with no geometry: ", jp->line); json_context(j); @@ -373,7 +378,7 @@ std::vector parse_layers(FILE *fp, int z, unsigned x, unsigned y, int feature.type = mb_geometry[t]; feature.geometry = to_feature(dv); - json_object_ptr id = json_hash_get(j, "id"); + json_object *id = json_hash_get(j, "id"); if (id != nullptr && id->type == JSON_NUMBER) { feature.id = id->number(); if (id->large_unsigned() > 0) { @@ -384,7 +389,7 @@ std::vector parse_layers(FILE *fp, int z, unsigned x, unsigned y, int if (properties->type == JSON_HASH) { for (const auto &e : properties->entries()) { - serial_val sv = stringify_value(e.value, "Filter output", jp->line, j); + serial_val sv = stringify_value(e.value.get(), "Filter output", jp->line, j); // Nulls can be excluded here because this is the postfilter // and it is nearly time to create the vector representation @@ -398,6 +403,8 @@ std::vector parse_layers(FILE *fp, int z, unsigned x, unsigned y, int l->second.features.push_back(feature); } + + json_free(j); } std::vector final; diff --git a/read_json.hpp b/read_json.hpp index 4b8f9fb8..f254e354 100644 --- a/read_json.hpp +++ b/read_json.hpp @@ -10,10 +10,10 @@ extern const char *geometry_names[GEOM_TYPES]; extern int geometry_within[GEOM_TYPES]; extern int mb_geometry[GEOM_TYPES]; -void json_context(json_object_ptr j); -void parse_coordinates(int t, json_object_ptr j, drawvec &out, int op, const char *fname, int line, json_object_ptr feature); -std::pair parse_geometry(json_object_ptr geometry, json_pull_ptr jp, json_object_ptr j, +void json_context(json_object *j); +void parse_coordinates(int t, json_object *j, drawvec &out, int op, const char *fname, int line, json_object *feature); +std::pair parse_geometry(json_object *geometry, json_pull_ptr &jp, json_object *j, int z, int x, int y, long long extent, bool fix_longitudes, bool mvt_style); std::vector parse_layers(FILE *fp, int z, unsigned x, unsigned y, int extent, bool fix_longitudes); -serial_val stringify_value(json_object_ptr value, const char *reading, int line, json_object_ptr feature); +serial_val stringify_value(json_object *value, const char *reading, int line, json_object *feature); diff --git a/tile-join.cpp b/tile-join.cpp index b729230c..fec371aa 100644 --- a/tile-join.cpp +++ b/tile-join.cpp @@ -89,7 +89,7 @@ struct arg { std::set *keep_layers = NULL; std::set *remove_layers = NULL; int ifmatched = 0; - json_object_ptr filter; + json_object *filter = NULL; struct tileset_reader *readers = NULL; double minlat, minlon; @@ -97,7 +97,7 @@ struct arg { double minlon2, maxlon2; }; -void append_tile(std::string message, int z, unsigned x, unsigned y, std::map &layermap, std::vector &header, std::map> &mapping, sqlite3 * /* db */, std::set &exclude, std::set &include, std::set &keep_layers, std::set &remove_layers, int ifmatched, mvt_tile &outtile, json_object_ptr filter, struct arg *a) { +void append_tile(std::string message, int z, unsigned x, unsigned y, std::map &layermap, std::vector &header, std::map> &mapping, sqlite3 * /* db */, std::set &exclude, std::set &include, std::set &keep_layers, std::set &remove_layers, int ifmatched, mvt_tile &outtile, json_object *filter, struct arg *a) { mvt_tile tile; int features_added = 0; bool was_compressed; @@ -891,7 +891,7 @@ void *join_worker(void *v) { return NULL; } -void dispatch_tasks(std::map> &tasks, std::vector> &layermaps, sqlite3 *outdb, const char *outdir, std::vector &header, std::map> &mapping, sqlite3 *db, std::set &exclude, std::set &include, int ifmatched, std::set &keep_layers, std::set &remove_layers, json_object_ptr filter, struct tileset_reader *readers, double *minlat, double *minlon, double *maxlat, double *maxlon, double *minlon2, double *maxlon2) { +void dispatch_tasks(std::map> &tasks, std::vector> &layermaps, sqlite3 *outdb, const char *outdir, std::vector &header, std::map> &mapping, sqlite3 *db, std::set &exclude, std::set &include, int ifmatched, std::set &keep_layers, std::set &remove_layers, json_object *filter, struct tileset_reader *readers, double *minlat, double *minlon, double *maxlat, double *maxlon, double *minlon2, double *maxlon2) { pthread_t pthreads[CPUS]; std::vector args; @@ -970,7 +970,7 @@ void handle_strategies(const unsigned char *s, std::vector *st) { if (o != nullptr && o->type == JSON_ARRAY) { for (size_t i = 0; i < o->array().size(); i++) { - json_object_ptr h = o->array()[i]; + const json_object_ptr &h = o->array()[i]; if (h->type == JSON_HASH) { size_t j = 0; for (const auto &kv : h->entries()) { @@ -1013,12 +1013,12 @@ void handle_strategies(const unsigned char *s, std::vector *st) { } } -void handle_vector_layers(json_object_ptr vector_layers, std::map &layermap, std::map &attribute_descriptions) { +void handle_vector_layers(json_object *vector_layers, std::map &layermap, std::map &attribute_descriptions) { if (vector_layers != nullptr && vector_layers->type == JSON_ARRAY) { for (size_t i = 0; i < vector_layers->array().size(); i++) { if (vector_layers->array()[i]->type == JSON_HASH) { - json_object_ptr id = json_hash_get(vector_layers->array()[i], "id"); - json_object_ptr desc = json_hash_get(vector_layers->array()[i], "description"); + json_object *id = json_hash_get(vector_layers->array()[i].get(), "id"); + json_object *desc = json_hash_get(vector_layers->array()[i].get(), "description"); if (id != nullptr && desc != nullptr && id->type == JSON_STRING && desc->type == JSON_STRING) { const std::string &sid = id->string(); @@ -1032,7 +1032,7 @@ void handle_vector_layers(json_object_ptr vector_layers, std::maparray()[i], "fields"); + json_object *fields = json_hash_get(vector_layers->array()[i].get(), "fields"); if (fields != nullptr && fields->type == JSON_HASH) { for (const auto &e : fields->entries()) { if (e.key != nullptr && e.key->type == JSON_STRING && @@ -1053,7 +1053,7 @@ void handle_vector_layers(json_object_ptr vector_layers, std::map &layermap, sqlite3 *outdb, const char *outdir, struct stats *st, std::vector &header, std::map> &mapping, sqlite3 *db, std::set &exclude, std::set &include, int ifmatched, std::string &attribution, std::string &description, std::set &keep_layers, std::set &remove_layers, std::string &name, json_object_ptr filter, std::map &attribute_descriptions, std::string &generator_options, std::vector *strategies) { +void decode(struct tileset_reader *readers, std::map &layermap, sqlite3 *outdb, const char *outdir, struct stats *st, std::vector &header, std::map> &mapping, sqlite3 *db, std::set &exclude, std::set &include, int ifmatched, std::string &attribution, std::string &description, std::set &keep_layers, std::set &remove_layers, std::string &name, json_object *filter, std::map &attribute_descriptions, std::string &generator_options, std::vector *strategies) { std::vector> layermaps; for (size_t i = 0; i < CPUS; i++) { layermaps.push_back(std::map()); @@ -1207,7 +1207,7 @@ void decode(struct tileset_reader *readers, std::maptype == JSON_HASH) { - json_object_ptr vector_layers = json_hash_get(o, "vector_layers"); + json_object *vector_layers = json_hash_get(o, "vector_layers"); handle_vector_layers(vector_layers, layermap, attribute_descriptions); } @@ -1591,7 +1591,7 @@ int main(int argc, char **argv) { std::string generator_options; std::vector strategies; - decode(readers, layermap, outdb, out_dir, &st, header, mapping, db, exclude, include, ifmatched, attribution, description, keep_layers, remove_layers, name, filter, attribute_descriptions, generator_options, &strategies); + decode(readers, layermap, outdb, out_dir, &st, header, mapping, db, exclude, include, ifmatched, attribution, description, keep_layers, remove_layers, name, filter.get(), attribute_descriptions, generator_options, &strategies); if (set_attribution.size() != 0) { attribution = set_attribution; diff --git a/tile.cpp b/tile.cpp index 309d2e92..fd03a7e9 100644 --- a/tile.cpp +++ b/tile.cpp @@ -941,7 +941,7 @@ struct write_tile_args { bool still_dropping = false; int wrote_zoom = 0; size_t tiling_seg = 0; - json_object_ptr filter; + json_object *filter = NULL; std::vector const *unidecode_data; std::atomic *dropped_count = NULL; atomic_strategy *strategy = NULL; @@ -1102,7 +1102,7 @@ struct next_feature_state { // This function is called repeatedly from write_tile() to retrieve the next feature // from the input stream. If the stream is at an end, it returns a feature with the // geometry type set to -2. -static serial_feature next_feature(decompressor *geoms, std::atomic *geompos_in, int z, unsigned tx, unsigned ty, unsigned *initial_x, unsigned *initial_y, long long *original_features, long long *unclipped_features, int nextzoom, int maxzoom, int minzoom, int max_zoom_increment, size_t pass, std::atomic *along, long long alongminus, int buffer, std::atomic *within, compressor **geomfile, std::atomic *geompos, long long start_geompos[], std::atomic *oprogress, double todo, const char *fname, int child_shards, json_object_ptr filter, const char *global_stringpool, long long *pool_off, std::vector> *layer_unmaps, bool first_time, bool compressed, multiplier_state *multiplier_state, std::shared_ptr &tile_stringpool, std::vector const &unidecode_data, next_feature_state &next_feature_state, double droprate) { +static serial_feature next_feature(decompressor *geoms, std::atomic *geompos_in, int z, unsigned tx, unsigned ty, unsigned *initial_x, unsigned *initial_y, long long *original_features, long long *unclipped_features, int nextzoom, int maxzoom, int minzoom, int max_zoom_increment, size_t pass, std::atomic *along, long long alongminus, int buffer, std::atomic *within, compressor **geomfile, std::atomic *geompos, long long start_geompos[], std::atomic *oprogress, double todo, const char *fname, int child_shards, json_object *filter, const char *global_stringpool, long long *pool_off, std::vector> *layer_unmaps, bool first_time, bool compressed, multiplier_state *multiplier_state, std::shared_ptr &tile_stringpool, std::vector const &unidecode_data, next_feature_state &next_feature_state, double droprate) { double extra_multiplier_zooms = log(retain_points_multiplier) / log(droprate); while (1) { @@ -1350,7 +1350,7 @@ struct run_prefilter_args { char *global_stringpool = NULL; long long *pool_off = NULL; FILE *prefilter_fp = NULL; - json_object_ptr filter; + json_object *filter = NULL; std::vector const *unidecode_data; bool first_time = false; bool compressed = false; @@ -1641,7 +1641,7 @@ void skip_tile(decompressor *geoms, std::atomic *geompos_in, bool com } } -long long write_tile(decompressor *geoms, std::atomic *geompos_in, char *global_stringpool, int z, const unsigned tx, const unsigned ty, const int detail, int min_detail, sqlite3 *outdb, const char *outdir, int buffer, const char *fname, compressor **geomfile, std::atomic *geompos, int minzoom, int maxzoom, double todo, std::atomic *along, long long alongminus, double gamma, int child_shards, long long *pool_off, unsigned *initial_x, unsigned *initial_y, std::atomic *running, double simplification, std::vector> *layermaps, std::vector> *layer_unmaps, size_t tiling_seg, size_t pass, unsigned long long mingap, long long minextent, unsigned long long mindrop_sequence, double minattribute, const char *prefilter, const char *postfilter, json_object_ptr filter, write_tile_args *arg, atomic_strategy *strategy_out, bool compressed_input, node *shared_nodes_map, size_t nodepos, std::string const &shared_nodes_bloom, std::vector const &unidecode_data, long long estimated_complexity, std::set &skip_children_out) { +long long write_tile(decompressor *geoms, std::atomic *geompos_in, char *global_stringpool, int z, const unsigned tx, const unsigned ty, const int detail, int min_detail, sqlite3 *outdb, const char *outdir, int buffer, const char *fname, compressor **geomfile, std::atomic *geompos, int minzoom, int maxzoom, double todo, std::atomic *along, long long alongminus, double gamma, int child_shards, long long *pool_off, unsigned *initial_x, unsigned *initial_y, std::atomic *running, double simplification, std::vector> *layermaps, std::vector> *layer_unmaps, size_t tiling_seg, size_t pass, unsigned long long mingap, long long minextent, unsigned long long mindrop_sequence, double minattribute, const char *prefilter, const char *postfilter, json_object *filter, write_tile_args *arg, atomic_strategy *strategy_out, bool compressed_input, node *shared_nodes_map, size_t nodepos, std::string const &shared_nodes_bloom, std::vector const &unidecode_data, long long estimated_complexity, std::set &skip_children_out) { double merge_fraction = 1; double mingap_fraction = 1; double minextent_fraction = 1; @@ -3213,7 +3213,7 @@ exit(EXIT_IMPOSSIBLE); return err_or_null; } -int traverse_zooms(int *geomfd, off_t *geom_size, char *global_stringpool, std::atomic *midx, std::atomic *midy, int &maxzoom, int minzoom, sqlite3 *outdb, const char *outdir, int buffer, const char *fname, const char *tmpdir, double gamma, int full_detail, int low_detail, int min_detail, long long *pool_off, unsigned *initial_x, unsigned *initial_y, double simplification, double maxzoom_simplification, std::vector> &layermaps, const char *prefilter, const char *postfilter, std::unordered_map const *attribute_accum, json_object_ptr filter, std::vector &strategies, int iz, node *shared_nodes_map, size_t nodepos, std::string const &shared_nodes_bloom, int basezoom, double droprate, std::vector const &unidecode_data, std::string const *drop_by_attribute_as_needed_attribute, bool drop_by_attribute_descending) { +int traverse_zooms(int *geomfd, off_t *geom_size, char *global_stringpool, std::atomic *midx, std::atomic *midy, int &maxzoom, int minzoom, sqlite3 *outdb, const char *outdir, int buffer, const char *fname, const char *tmpdir, double gamma, int full_detail, int low_detail, int min_detail, long long *pool_off, unsigned *initial_x, unsigned *initial_y, double simplification, double maxzoom_simplification, std::vector> &layermaps, const char *prefilter, const char *postfilter, std::unordered_map const *attribute_accum, json_object *filter, std::vector &strategies, int iz, node *shared_nodes_map, size_t nodepos, std::string const &shared_nodes_bloom, int basezoom, double droprate, std::vector const &unidecode_data, std::string const *drop_by_attribute_as_needed_attribute, bool drop_by_attribute_descending) { last_progress = 0; // The existing layermaps are one table per input thread. diff --git a/tile.hpp b/tile.hpp index e9c7d42a..8b72e3fb 100644 --- a/tile.hpp +++ b/tile.hpp @@ -62,7 +62,7 @@ struct strategy { // long long write_tile(char **geom, char *stringpool, unsigned *file_bbox, int z, unsigned x, unsigned y, int detail, int min_detail, int basezoom, sqlite3 *outdb, const char *outdir, double droprate, int buffer, const char *fname, FILE **geomfile, int file_minzoom, int file_maxzoom, double todo, char *geomstart, long long along, double gamma, int nlayers, std::atomic *strategy); -int traverse_zooms(int *geomfd, off_t *geom_size, char *stringpool, std::atomic *midx, std::atomic *midy, int &maxzoom, int minzoom, sqlite3 *outdb, const char *outdir, int buffer, const char *fname, const char *tmpdir, double gamma, int full_detail, int low_detail, int min_detail, long long *pool_off, unsigned *initial_x, unsigned *initial_y, double simplification, double maxzoom_simplification, std::vector > &layermap, const char *prefilter, const char *postfilter, std::unordered_map const *attribute_accum, json_object_ptr filter, std::vector &strategies, int iz, struct node *shared_nodes_map, size_t nodepos, std::string const &shared_nodes_bloom, int basezoom, double droprate, std::vector const &unidecode_data, std::string const *drop_by_attribute_as_needed_attribute, bool drop_by_attribute_descending); +int traverse_zooms(int *geomfd, off_t *geom_size, char *stringpool, std::atomic *midx, std::atomic *midy, int &maxzoom, int minzoom, sqlite3 *outdb, const char *outdir, int buffer, const char *fname, const char *tmpdir, double gamma, int full_detail, int low_detail, int min_detail, long long *pool_off, unsigned *initial_x, unsigned *initial_y, double simplification, double maxzoom_simplification, std::vector > &layermap, const char *prefilter, const char *postfilter, std::unordered_map const *attribute_accum, json_object *filter, std::vector &strategies, int iz, struct node *shared_nodes_map, size_t nodepos, std::string const &shared_nodes_bloom, int basezoom, double droprate, std::vector const &unidecode_data, std::string const *drop_by_attribute_as_needed_attribute, bool drop_by_attribute_descending); int manage_gap(unsigned long long index, unsigned long long *previndex, double scale, double gamma, double *gap); diff --git a/unit.cpp b/unit.cpp index bbd1a6d2..e30ae227 100644 --- a/unit.cpp +++ b/unit.cpp @@ -174,10 +174,10 @@ TEST_CASE("jsonpull surrogate-pair regression", "[jsonpull][surrogate]") { TEST_CASE("json_free prunes a subtree from its parent", "[jsonpull][memory]") { json_pull_ptr jp = json_begin_string("[[1, 2], [3, 4], [5, 6]]"); - json_object_ptr outer; + json_object *outer = nullptr; int arrays_seen = 0; - json_object_ptr j; + json_object *j; while ((j = json_read(jp)) != nullptr) { if (j->type != JSON_ARRAY) { continue; @@ -190,7 +190,8 @@ TEST_CASE("json_free prunes a subtree from its parent", "[jsonpull][memory]") { REQUIRE(j->array()[1]->number() == 4); json_free(j); } else if (j->parent == nullptr) { - // The completed outer array. + // The completed outer array; the parser still owns it + // via jp->root, so the borrowed pointer stays valid. outer = j; break; } @@ -215,21 +216,33 @@ TEST_CASE("json_free prunes a subtree from its parent", "[jsonpull][memory]") { // The companion case to the pruning test above: in a line-delimited // stream, each feature returned by json_read is a top-level value -// with no parent, but the parser still co-owns it via jp->root. +// with no parent, but the parser still owns it via jp->root. // json_free must drop that parser reference too, otherwise the -// just-serialized feature stays in memory until the next feature -// starts parsing. +// just-serialized feature would sit in memory until the next feature +// started parsing. Under the unique_ptr ownership model, the only +// owner is jp->root, so verifying that jp->root is empty after the +// json_free call is also a guarantee that the subtree itself has +// been destroyed. TEST_CASE("json_free releases a top-level value held by the parser", "[jsonpull][memory]") { - std::weak_ptr observer; - json_pull_ptr jp = json_begin_string(R"({"a": 1, "b": [2, 3]})"); - json_object_ptr j = json_read_tree(jp); - REQUIRE(j != nullptr); - REQUIRE(j->type == JSON_HASH); - REQUIRE(j->parent == nullptr); - observer = j; - json_free(j); + // json_read streams atoms first (1, 2, 3, [2,3], ...); the top-level + // hash is returned by the final `}` token. + json_object *top = nullptr; + json_object *j; + while ((j = json_read(jp)) != nullptr) { + if (j->parent == nullptr) { + top = j; + break; + } + } - REQUIRE(observer.expired()); + REQUIRE(top != nullptr); + REQUIRE(top->type == JSON_HASH); + REQUIRE(jp->root.get() == top); + + json_free(top); + // top is dangling now; do not dereference. + + REQUIRE(jp->root == nullptr); }