Commit 3ab698bc authored by T.J. Corona's avatar T.J. Corona

Allow ReferenceItems to release their references

Originally, ReferenceItems cached shared pointers to resources and
components. The use of shared pointers resulted in the undesired
behavior of resources outliving their presence in the resource
manager. This MR adds an option to ReferenceItems (disabled by
default) to use shared pointers to store their references. By default,
ReferenceItems now use weak pointers, allowing the referenced
resource/component to go out of scope.
parent 7d242522
## Reference Items do not hold references by default
Originally, ReferenceItems cached shared pointers to resources and
components. The use of shared pointers resulted in the undesired
behavior of resources outliving their presence in the resource
manager. There is now an option for ReferenceItems (disabled by
default) to use shared pointers to store their references. By default,
ReferenceItems now use weak pointers, allowing the referenced
resource/component to go out of scope.
This diff is collapsed.
......@@ -15,6 +15,7 @@
#include "smtk/common/UUID.h"
#include "smtk/resource/Lock.h"
#include <iterator>
#include <vector>
namespace smtk
......@@ -48,9 +49,60 @@ class ReferenceItemDefinition;
*/
class SMTKCORE_EXPORT ReferenceItem : public Item
{
public:
using PersistentObjectPtr = smtk::resource::PersistentObjectPtr;
using const_iterator = std::vector<smtk::resource::PersistentObjectPtr>::const_iterator;
public:
/**\brief An iterator for references held by a ReferenceItem.
*
* Iterators into ReferenceItem will always dereference to a
* PersistentObjectPtr, regardless of the underlying storage mechanism used
* to hold the reference.
*/
class SMTKCORE_EXPORT const_iterator
{
friend class ReferenceItem;
public:
typedef const_iterator self_type;
typedef std::random_access_iterator_tag iterator_category;
typedef const smtk::resource::PersistentObjectPtr value_type;
typedef value_type reference;
typedef value_type pointer;
typedef std::ptrdiff_t difference_type;
const_iterator();
const_iterator(const const_iterator& it);
~const_iterator();
const_iterator& operator=(const const_iterator& it);
const_iterator& operator++();
const_iterator& operator--();
const_iterator operator++(int);
const_iterator operator--(int);
const_iterator operator+(const difference_type& d) const;
const_iterator operator-(const difference_type& d) const;
reference operator*() const;
pointer operator->() const;
reference operator[](const difference_type& d);
friend difference_type SMTKCORE_EXPORT operator-(const const_iterator&, const const_iterator&);
friend bool SMTKCORE_EXPORT operator<(const const_iterator& it1, const const_iterator& it2);
friend bool SMTKCORE_EXPORT operator>(const const_iterator& it1, const const_iterator& it2);
friend bool SMTKCORE_EXPORT operator<=(const const_iterator& it1, const const_iterator& it2);
friend bool SMTKCORE_EXPORT operator>=(const const_iterator& it1, const const_iterator& it2);
friend bool SMTKCORE_EXPORT operator==(const const_iterator& it1, const const_iterator& it2);
friend bool SMTKCORE_EXPORT operator!=(const const_iterator& it1, const const_iterator& it2);
private:
struct CacheIterator;
std::unique_ptr<CacheIterator> m_cacheIterator;
};
/// A Key is a pair of UUIDs. the First UUID is the id of the resource link,
/// and the second one is the id of the component link.
......@@ -58,9 +110,12 @@ public:
smtkTypeMacro(ReferenceItem);
smtkSuperclassMacro(Item);
/// Destructor
ReferenceItem(const ReferenceItem&);
~ReferenceItem() override;
ReferenceItem& operator=(const ReferenceItem&);
/// Indicate we are a reference to a persistent object.
Item::Type type() const override { return Item::ReferenceType; }
......@@ -152,15 +207,18 @@ public:
*/
bool setObjectValue(PersistentObjectPtr val);
/// Set the \a i-th value to the given item. This method does no checking to see if \a i is valid.
//bool setObjectValue(std::size_t i, PersistentObjectPtr val);
bool setObjectValue(std::size_t i, PersistentObjectPtr val);
template <typename I>
bool setObjectValues(I vbegin, I vend, std::size_t offset = 0);
bool setObjectValues(
I vbegin, I vend, typename std::iterator_traits<I>::difference_type offset = 0);
template <typename I>
bool appendObjectValues(I vbegin, I vend);
template <typename I, typename T>
bool setValuesVia(I vbegin, I vend, const T& converter, std::size_t offset = 0);
bool setValuesVia(I vbegin, I vend, const T& converter,
typename std::iterator_traits<I>::difference_type offset = 0);
template <typename I, typename T>
bool appendValuesVia(I vbegin, I vend, const T& converter);
......@@ -257,20 +315,27 @@ protected:
/// Resolve the object pointers by accessing them using their associated keys.
/// Return true if all object pointers were successfully resolved.
bool resolve();
bool resolve() const;
/// Construct a link between the attribute that owns this item and \a val.
Key linkTo(PersistentObjectPtr val);
std::vector<PersistentObjectPtr> m_values;
std::vector<Key> m_keys;
private:
void assignToCache(std::size_t i, const PersistentObjectPtr& obj) const;
void appendToCache(const PersistentObjectPtr& obj) const;
struct Cache;
mutable std::unique_ptr<Cache> m_cache;
};
template <typename I>
bool ReferenceItem::setObjectValues(I vbegin, I vend, std::size_t offset)
bool ReferenceItem::setObjectValues(
I vbegin, I vend, typename std::iterator_traits<I>::difference_type offset)
{
bool ok = false;
std::size_t num = vend - vbegin + offset;
std::size_t num = std::distance(vbegin, vend) + offset;
if (this->setNumberOfValues(num))
{
ok = true;
......@@ -299,10 +364,11 @@ bool ReferenceItem::appendObjectValues(I vbegin, I vend)
}
template <typename I, typename T>
bool ReferenceItem::setValuesVia(I vbegin, I vend, const T& converter, std::size_t offset)
bool ReferenceItem::setValuesVia(
I vbegin, I vend, const T& converter, typename std::iterator_traits<I>::difference_type offset)
{
bool ok = false;
std::size_t num = vend - vbegin + offset;
std::size_t num = std::distance(vbegin, vend) + offset;
if (this->setNumberOfValues(num))
{
ok = true;
......
......@@ -36,6 +36,7 @@ ReferenceItemDefinition::ReferenceItemDefinition(const std::string& sname)
m_maxNumberOfValues = 0;
m_lockType = smtk::resource::LockType::Write;
m_role = smtk::attribute::Resource::ReferenceRole;
m_holdReference = false;
}
ReferenceItemDefinition::~ReferenceItemDefinition()
......
......@@ -92,6 +92,12 @@ public:
/// smtk::attribute::Resource::ReferenceRole.
smtk::resource::Links::RoleType role() const { return m_role; }
/// Set/Get a flag to determine whether the ReferenceItem should keep an
/// assigned reference in memory (i.e. shared_ptr vs weak_ptr to the
/// reference).
void setHoldReference(bool choice) { m_holdReference = choice; }
bool holdReference() const { return m_holdReference; }
smtk::attribute::ItemPtr buildItem(Attribute* owningAttribute, int itemPosition) const override;
smtk::attribute::ItemPtr buildItem(Item* owner, int itemPos, int subGroupPosition) const override;
......@@ -127,6 +133,7 @@ protected:
std::multimap<std::string, std::string> m_acceptable;
smtk::resource::LockType m_lockType;
smtk::resource::Links::RoleType m_role;
bool m_holdReference;
};
} // namespace attribute
......
......@@ -61,6 +61,10 @@ SMTKCORE_EXPORT void to_json(
}
j["ReferenceLabels"] = valueLabel;
}
if (defPtr->holdReference())
{
j["HoldReference"] = true;
}
}
SMTKCORE_EXPORT void from_json(
......@@ -137,6 +141,13 @@ SMTKCORE_EXPORT void from_json(
{
}
}
try
{
defPtr->setHoldReference(j.at("HoldReference"));
}
catch (std::exception& /*e*/)
{
}
}
}
}
......@@ -20,7 +20,7 @@
<AttDef Type="result(read)" BaseType="result">
<ItemDefinitions>
<Resource Name="resource">
<Resource Name="resource" HoldReference="true">
<Accepts>
<Resource Name="smtk::attribute::Resource"/>
</Accepts>
......
......@@ -30,6 +30,8 @@ PySharedPtrClass< smtk::attribute::ReferenceItemDefinition, smtk::attribute::Ite
.def("buildItem", (smtk::attribute::ItemPtr (smtk::attribute::ReferenceItemDefinition::*)(::smtk::attribute::Item *, int, int) const) &smtk::attribute::ReferenceItemDefinition::buildItem, py::arg("owner"), py::arg("itemPos"), py::arg("subGroupPosition"))
.def("createCopy", &smtk::attribute::ReferenceItemDefinition::createCopy, py::arg("info"))
.def("hasValueLabels", &smtk::attribute::ReferenceItemDefinition::hasValueLabels)
.def("holdReference", (bool (smtk::attribute::ReferenceItemDefinition::*)() const) &smtk::attribute::ReferenceItemDefinition::holdReference)
.def("setHoldReference", (void (smtk::attribute::ReferenceItemDefinition::*)(bool)) &smtk::attribute::ReferenceItemDefinition::setHoldReference)
.def("isExtensible", &smtk::attribute::ReferenceItemDefinition::isExtensible)
.def("isValueValid", &smtk::attribute::ReferenceItemDefinition::isValueValid, py::arg("entity"))
.def("lockType", &smtk::attribute::ReferenceItemDefinition::lockType)
......
......@@ -16,6 +16,7 @@ set(attributeTests
attributeAutoNamingTest
attributeReferencingTest
categoryTest
referenceItemStorageTest
)
set(basicAttributeXMLWriterTest_ARGS
"${CMAKE_BINARY_DIR}/Testing/Temporary/basicAttributeXMLWriterTest.xml"
......
//=========================================================================
// Copyright (c) Kitware, Inc.
// All rights reserved.
// See LICENSE.txt for details.
//
// This software is distributed WITHOUT ANY WARRANTY; without even
// the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
// PURPOSE. See the above copyright notice for more information.
//=========================================================================
#include "smtk/attribute/Attribute.h"
#include "smtk/attribute/Definition.h"
#include "smtk/attribute/Resource.h"
#include "smtk/attribute/ResourceItem.h"
#include "smtk/io/AttributeReader.h"
#include "smtk/io/Logger.h"
#include <iostream>
namespace
{
const char* testInput =
"<?xml version=\"1.0\" encoding=\"utf-8\" ?> "
"<SMTK_AttributeResource Version=\"3\"> "
" <Definitions> "
" <AttDef Type=\"att\" BaseType=\"\"> "
" <ItemDefinitions> "
" <Resource Name=\"holdReference\" NumberOfRequiredValues=\"1\" "
" HoldReference=\"1\"> "
" <Accepts> "
" <Resource Name=\"smtk::attribute::Resource\"/> "
" </Accepts> "
" </Resource> "
" <Resource Name=\"dropReference\" NumberOfRequiredValues=\"1\" "
" HoldReference=\"0\"> "
" <Accepts> "
" <Resource Name=\"smtk::attribute::Resource\"/> "
" </Accepts> "
" </Resource> "
" </ItemDefinitions> "
" </AttDef> "
" </Definitions> "
"</SMTK_AttributeResource> ";
}
int main()
{
// Create an attribute resource as described by the above preamble
smtk::attribute::ResourcePtr resource = smtk::attribute::Resource::create();
{
smtk::io::Logger logger;
smtk::io::AttributeReader reader;
if (reader.readContents(resource, testInput, logger))
{
std::cerr << "Encountered Errors while reading input data\n";
std::cerr << logger.convertToString();
return -2;
}
}
// Create instances of resource items that
smtk::attribute::AttributePtr att = resource->createAttribute("att");
smtk::attribute::ResourceItemPtr holdReference = att->findResource("holdReference");
smtk::attribute::ResourceItemPtr dropReference = att->findResource("dropReference");
{
smtk::attribute::ResourcePtr heldResource = smtk::attribute::Resource::create();
holdReference->setValue(heldResource);
dropReference->setValue(heldResource);
if (holdReference->value() == nullptr || dropReference->value() == nullptr)
{
std::cerr << "Encountered Errors while assigning reference item\n";
return -2;
}
}
if (holdReference->value() == nullptr || dropReference->value() == nullptr)
{
std::cerr << "Resource was not held by reference item\n";
return -1;
}
holdReference->reset();
if (dropReference->value() != nullptr)
{
std::cerr << "Resource was incorrectly held by reference item\n";
return -1;
}
return 0;
}
......@@ -30,7 +30,7 @@
<include href="smtk/operation/Result.xml"/>
<AttDef Type="result(triangulate faces)" BaseType="result">
<ItemDefinitions>
<Resource Name="meshresource" NumberOfRequiredValues="1">
<Resource Name="meshresource" NumberOfRequiredValues="1" HoldReference="true">
<Accepts><Resource Name="smtk::mesh::Resource"/></Accepts>
</Resource>
<Component Name="mesh_created" NumberOfRequiredValues="1" Extensible="true">
......
......@@ -556,6 +556,12 @@ void XmlDocV3Parser::processReferenceDef(pugi::xml_node& node,
}
}
xatt = node.attribute("HoldReference");
if (xatt)
{
idef->setHoldReference(xatt.as_bool());
}
xatt = node.attribute("NumberOfRequiredValues");
if (xatt)
{
......
......@@ -506,6 +506,11 @@ void XmlV3StringWriter::processReferenceDefCommon(pugi::xml_node& node,
.set_value((idef->lockType() == smtk::resource::LockType::DoNotLock ? "DoNotLock" : "Read"));
}
if (idef->holdReference())
{
node.append_attribute("HoldReference") = true;
}
node.append_attribute("NumberOfRequiredValues") =
static_cast<unsigned int>(idef->numberOfRequiredValues());
if (idef->isExtensible())
......
......@@ -27,7 +27,7 @@
<AttDef Type="result(import)" BaseType="result">
<ItemDefinitions>
<Resource Name="resource">
<Resource Name="resource" HoldReference="true">
<Accepts>
<Resource Name="smtk::mesh::Resource"/>
</Accepts>
......
......@@ -28,7 +28,7 @@
<AttDef Type="result(read)" BaseType="result">
<ItemDefinitions>
<Resource Name="resource">
<Resource Name="resource" HoldReference="true">
<Accepts>
<Resource Name="smtk::mesh::Resource"/>
</Accepts>
......
......@@ -20,7 +20,7 @@
<include href="smtk/operation/Result.xml"/>
<AttDef Type="result(create resource)" BaseType="result">
<ItemDefinitions>
<Resource Name="resource" IsEnabledByDefault="true"></Resource>
<Resource Name="resource" IsEnabledByDefault="true" HoldReference="true"></Resource>
</ItemDefinitions>
</AttDef>
</Definitions>
......
......@@ -4,8 +4,8 @@
</Int>
<String Name="log" Optional="True" NumberOfRequiredValues="0" Extensible="True">
</String>
<Component Name="created" NumberOfRequiredValues="0" Extensible="1"/>
<Component Name="modified" NumberOfRequiredValues="0" Extensible="1"/>
<Component Name="expunged" NumberOfRequiredValues="0" Extensible="1"/>
<Component Name="created" NumberOfRequiredValues="0" Extensible="1" HoldReference="1"/>
<Component Name="modified" NumberOfRequiredValues="0" Extensible="1" HoldReference="1"/>
<Component Name="expunged" NumberOfRequiredValues="0" Extensible="1" HoldReference="1"/>
</ItemDefinitions>
</AttDef>
......@@ -214,6 +214,7 @@ ImportResource::Specification ImportResource::createSpecification()
auto resourceDef = smtk::attribute::ResourceItemDefinition::New("resource");
resourceDef->setNumberOfRequiredValues(0);
resourceDef->setIsExtensible(true);
resourceDef->setHoldReference(true);
resultDef->addItemDefinition(resourceDef);
return spec;
......
......@@ -24,7 +24,8 @@
<include href="smtk/operation/Result.xml"/>
<AttDef Type="result(read resource)" BaseType="result">
<ItemDefinitions>
<Resource Name="resource" NumberOfRequiredValues="0" Extensible="true"></Resource>
<Resource Name="resource" NumberOfRequiredValues="0"
Extensible="true" HoldReference="true"></Resource>
</ItemDefinitions>
</AttDef>
</Definitions>
......
......@@ -93,7 +93,7 @@
<ItemDefinitions>
<!-- The model imported from the file. -->
<Resource Name="resource" Extensible="1">
<Resource Name="resource" Extensible="1" HoldReference="true">
<Accepts>
<Resource Name="smtk::session::discrete::Resource"/>
<Resource Name="smtk::mesh::Resource"/>
......
......@@ -16,7 +16,7 @@
<AttDef Type="result(legacy read)" BaseType="result">
<ItemDefinitions>
<Resource Name="resource">
<Resource Name="resource" HoldReference="true">
<Accepts>
<Resource Name="smtk::session::discrete::Resource"/>
</Accepts>
......
......@@ -45,9 +45,9 @@
<ItemDefinitions>
<!-- The model imported from the file. -->
<Resource Name="resource">
<Resource Name="resource" HoldReference="true">
<Accepts>
<Resource Name="smtk::session::discrete::Resource"/>
<Resource Name="smtk::session::discrete::Resource" HoldReference="true"/>
</Accepts>
</Resource>
......
......@@ -16,7 +16,7 @@
<AttDef Type="result(read)" BaseType="result">
<ItemDefinitions>
<Resource Name="resource">
<Resource Name="resource" HoldReference="true">
<Accepts>
<Resource Name="smtk::session::discrete::Resource"/>
</Accepts>
......
......@@ -113,7 +113,7 @@
<AttDef Type="result(createBackgroundDomain)" BaseType="result">
<ItemDefinitions>
<Resource Name="resource">
<Resource Name="resource" HoldReference="true">
<Accepts>
<Resource Name="smtk::session::mesh::Resource"/>
</Accepts>
......
......@@ -57,7 +57,7 @@
<ItemDefinitions>
<!-- The model imported from the file. -->
<Resource Name="resource">
<Resource Name="resource" HoldReference="true">
<Accepts>
<Resource Name="smtk::session::mesh::Resource"/>
</Accepts>
......
......@@ -16,7 +16,7 @@
<AttDef Type="result(read)" BaseType="result">
<ItemDefinitions>
<Resource Name="resource">
<Resource Name="resource" HoldReference="true">
<Accepts>
<Resource Name="smtk::session::mesh::Resource"/>
</Accepts>
......
......@@ -87,7 +87,7 @@
<Component Name="mesh_created" NumberOfRequiredValues="1">
<Accepts><Resource Name="smtk::session::multiscale::Session" Filter=""/></Accepts>
</Component>
<Resource Name="resource">
<Resource Name="resource" HoldReference="true">
<Accepts>
<Resource Name="smtk::session::multiscale::Resource"/>
</Accepts>
......
......@@ -38,7 +38,7 @@
<AttDef Type="result(create model)" BaseType="result">
<ItemDefinitions>
<!-- The created model is returned in the base result's "created" item. -->
<Resource Name="resource">
<Resource Name="resource" HoldReference="true">
<Accepts>
<Resource Name="smtk::session::oscillator::Resource"/>
</Accepts>
......
......@@ -16,7 +16,7 @@
<AttDef Type="result(read)" BaseType="result">
<ItemDefinitions>
<Resource Name="resource">
<Resource Name="resource" HoldReference="true">
<Accepts>
<Resource Name="smtk::session::oscillator::Resource"/>
</Accepts>
......
......@@ -157,7 +157,7 @@
<ItemDefinitions>
<!-- The model imported from the file. -->
<Resource Name="resource">
<Resource Name="resource" HoldReference="true">
<Accepts>
<Resource Name="smtk::session::polygon::Resource"/>
</Accepts>
......
......@@ -92,7 +92,7 @@
<ItemDefinitions>
<!-- The model imported from the file. -->
<Resource Name="resource">
<Resource Name="resource" HoldReference="true">
<Accepts>
<Resource Name="smtk::session::polygon::Resource"/>
</Accepts>
......
......@@ -16,7 +16,7 @@
<AttDef Type="result(legacy read)" BaseType="result">
<ItemDefinitions>
<Resource Name="resource">
<Resource Name="resource" HoldReference="true">
<Accepts>
<Resource Name="smtk::session::polygon::Resource"/>
</Accepts>
......
......@@ -16,7 +16,7 @@
<AttDef Type="result(read)" BaseType="result">
<ItemDefinitions>
<Resource Name="resource">
<Resource Name="resource" HoldReference="true">
<Accepts>
<Resource Name="smtk::session::polygon::Resource"/>
</Accepts>
......
......@@ -47,7 +47,7 @@
<AttDef Type="result(import)" BaseType="result">
<ItemDefinitions>
<Resource Name="resource">
<Resource Name="resource" HoldReference="true">
<Accepts>
<Resource Name="smtk::session::vtk::Resource"/>
</Accepts>
......
......@@ -16,7 +16,7 @@
<AttDef Type="result(legacy read)" BaseType="result">
<ItemDefinitions>
<Resource Name="resource">
<Resource Name="resource" HoldReference="true">
<Accepts>
<Resource Name="smtk::session::vtk::Resource"/>
</Accepts>
......
......@@ -16,7 +16,7 @@
<AttDef Type="result(read)" BaseType="result">
<ItemDefinitions>
<Resource Name="resource">
<Resource Name="resource" HoldReference="true">
<Accepts>
<Resource Name="smtk::session::vtk::Resource"/>
</Accepts>
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment