diff --git a/include/lsst/afw/table/AliasMap.h b/include/lsst/afw/table/AliasMap.h index 16d460a4e..49f9b45ec 100644 --- a/include/lsst/afw/table/AliasMap.h +++ b/include/lsst/afw/table/AliasMap.h @@ -38,14 +38,14 @@ class AliasMap final { public: // Create an empty AliasMap - AliasMap() : _internal(), _table(nullptr) {} + AliasMap() : _internal(), _table() {} /** * Deep-copy an AliasMap * * The new AliasMap will not be linked to any tables, even if other is. */ - AliasMap(AliasMap const& other) : _internal(other._internal), _table(nullptr) {} + AliasMap(AliasMap const& other) : _internal(other._internal), _table() {} // Delegate to copy-constructor for backwards compatibility AliasMap(AliasMap&& other) : AliasMap(other) {} @@ -124,7 +124,6 @@ class AliasMap final { private: friend class Schema; friend class SubSchema; - friend class BaseTable; // Internal in-place implementation of apply() void _apply(std::string& name) const; @@ -134,7 +133,7 @@ class AliasMap final { // Table to notify of any changes. We can't use a shared_ptr here because the Table needs to set // this in its own constructor, but the Table does guarantee that this pointer is either valid or // null. - BaseTable* _table; + std::weak_ptr _table; }; } // namespace table } // namespace afw diff --git a/include/lsst/afw/table/BaseTable.h b/include/lsst/afw/table/BaseTable.h index 1c86f2b3d..b0457063c 100644 --- a/include/lsst/afw/table/BaseTable.h +++ b/include/lsst/afw/table/BaseTable.h @@ -215,10 +215,7 @@ class BaseTable : public std::enable_shared_from_this { /// Construct from a schema. explicit BaseTable(Schema const& schema, std::shared_ptr metadata = nullptr); - /// Copy construct. - BaseTable(BaseTable const& other) : _schema(other._schema), _metadata(other._metadata) { - if (_metadata) _metadata = std::static_pointer_cast(_metadata->deepCopy()); - } + BaseTable(BaseTable const& other); // Delegate to copy-constructor for backwards compatibility BaseTable(BaseTable&& other) : BaseTable(other) {} diff --git a/src/table/AliasMap.cc b/src/table/AliasMap.cc index 25e65b246..49ac27af0 100644 --- a/src/table/AliasMap.cc +++ b/src/table/AliasMap.cc @@ -81,15 +81,17 @@ std::string AliasMap::get(std::string const& name) const { void AliasMap::set(std::string const& alias, std::string const& target) { _internal[alias] = target; - if (_table) { - _table->handleAliasChange(alias); + auto table = _table.lock(); + if (table) { + table->handleAliasChange(alias); } } bool AliasMap::erase(std::string const& alias) { bool result = _internal.erase(alias); - if (_table) { - _table->handleAliasChange(alias); + auto table = _table.lock(); + if (table) { + table->handleAliasChange(alias); } return result; } diff --git a/src/table/BaseTable.cc b/src/table/BaseTable.cc index 13477493a..7fb42d303 100644 --- a/src/table/BaseTable.cc +++ b/src/table/BaseTable.cc @@ -118,7 +118,9 @@ std::size_t BaseTable::getBufferSize() const { } std::shared_ptr BaseTable::make(Schema const &schema) { - return std::shared_ptr(new BaseTable(schema)); + std::shared_ptr table(new BaseTable(schema)); + table->getSchema().getAliasMap()->setTable(table); + return table; } Schema BaseTable::makeMinimalSchema() { return Schema(); } @@ -140,7 +142,9 @@ std::shared_ptr BaseTable::makeFitsWriter(fits::Fits *fitsfile, } std::shared_ptr BaseTable::_clone() const { - return std::shared_ptr(new BaseTable(*this)); + std::shared_ptr table(new BaseTable(*this)); + table->getSchema().getAliasMap()->setTable(table); + return table; } std::shared_ptr BaseTable::_makeRecord() { return constructRecord(); } @@ -149,10 +153,16 @@ BaseTable::BaseTable(Schema const &schema, std::shared_ptr_table = this; + _schema.getAliasMap()->getTable().reset(); } -BaseTable::~BaseTable() { _schema.getAliasMap()->_table = nullptr; } +BaseTable::BaseTable(BaseTable const &other) : _schema(other._schema), _metadata(other._metadata) { + _schema.disconnectAliases(); + _schema.getAliasMap()->getTable().reset(); + if (_metadata) _metadata = std::static_pointer_cast(_metadata->deepCopy()); +} + +BaseTable::~BaseTable() { _schema.getAliasMap()->getTable().reset(); } namespace { diff --git a/src/table/Exposure.cc b/src/table/Exposure.cc index 56220f7ae..db0b890d2 100644 --- a/src/table/Exposure.cc +++ b/src/table/Exposure.cc @@ -397,7 +397,9 @@ std::shared_ptr ExposureTable::make(Schema const &schema) { lsst::pex::exceptions::InvalidParameterError, "Schema for Exposure must contain at least the keys defined by makeMinimalSchema()."); } - return std::shared_ptr(new ExposureTable(schema)); + std::shared_ptr table(new ExposureTable(schema)); + table->getSchema().getAliasMap()->setTable(table); + return table; } ExposureTable::ExposureTable(Schema const &schema) : BaseTable(schema) {} @@ -429,7 +431,9 @@ std::shared_ptr ExposureTable::makeFitsWriter(fits::Fits *fitsfi } std::shared_ptr ExposureTable::_clone() const { - return std::shared_ptr(new ExposureTable(*this)); + std::shared_ptr table(new ExposureTable(*this)); + table->getSchema().getAliasMap()->setTable(table); + return table; } std::shared_ptr ExposureTable::_makeRecord() { diff --git a/src/table/Simple.cc b/src/table/Simple.cc index e99b75a6f..6bac3c021 100644 --- a/src/table/Simple.cc +++ b/src/table/Simple.cc @@ -78,7 +78,9 @@ std::shared_ptr SimpleTable::make(Schema const& schema, throw LSST_EXCEPT(lsst::pex::exceptions::InvalidParameterError, "Schema for Simple must contain at least the keys defined by makeMinimalSchema()."); } - return std::shared_ptr(new SimpleTable(schema, idFactory)); + std::shared_ptr table(new SimpleTable(schema, idFactory)); + table->getSchema().getAliasMap()->setTable(table); + return table; } SimpleTable::SimpleTable(Schema const& schema, std::shared_ptr const& idFactory) @@ -106,7 +108,9 @@ std::shared_ptr SimpleTable::makeFitsWriter(fits::Fits* fitsfile } std::shared_ptr SimpleTable::_clone() const { - return std::shared_ptr(new SimpleTable(*this)); + std::shared_ptr table(new SimpleTable(*this)); + table->getSchema().getAliasMap()->setTable(table); + return table; } std::shared_ptr SimpleTable::_makeRecord() { diff --git a/src/table/Source.cc b/src/table/Source.cc index 82bef4b35..b3eb3034c 100644 --- a/src/table/Source.cc +++ b/src/table/Source.cc @@ -403,7 +403,9 @@ std::shared_ptr SourceTable::make(Schema const &schema, throw LSST_EXCEPT(lsst::pex::exceptions::InvalidParameterError, "Schema for Source must contain at least the keys defined by getMinimalSchema()."); } - return std::shared_ptr(new SourceTable(schema, idFactory)); + std::shared_ptr table(new SourceTable(schema, idFactory)); + table->getSchema().getAliasMap()->setTable(table); + return table; } SourceTable::SourceTable(Schema const &schema, std::shared_ptr const &idFactory) @@ -435,7 +437,9 @@ std::shared_ptr SourceTable::makeFitsWriter(fits::Fits *fitsfile } std::shared_ptr SourceTable::_clone() const { - return std::shared_ptr(new SourceTable(*this)); + std::shared_ptr table(new SourceTable(*this)); + table->getSchema().getAliasMap()->setTable(table); + return table; } std::shared_ptr SourceTable::_makeRecord() { diff --git a/tests/test_tableAliases.cc b/tests/test_tableAliases.cc index 891561bba..bbd509414 100644 --- a/tests/test_tableAliases.cc +++ b/tests/test_tableAliases.cc @@ -31,7 +31,9 @@ class TestTable : public lsst::afw::table::BaseTable { mutable std::string lastAliasChanged; static std::shared_ptr make(lsst::afw::table::Schema const& schema) { - return std::make_shared(schema); + auto table = std::make_shared(schema); + table->getSchema().getAliasMap()->setTable(table); + return table; } explicit TestTable(lsst::afw::table::Schema const& schema) : lsst::afw::table::BaseTable(schema) {} @@ -44,7 +46,9 @@ class TestTable : public lsst::afw::table::BaseTable { void handleAliasChange(std::string const& alias) override { lastAliasChanged = alias; } std::shared_ptr _clone() const override { - return std::make_shared(*this); + auto table = std::make_shared(*this); + table->getSchema().getAliasMap()->setTable(table); + return table; } std::shared_ptr _makeRecord() override {