Skip to content

Commit

Permalink
simple lifetime check using nullptr
Browse files Browse the repository at this point in the history 
* when a mut::Morphology is deleted, and it resets the mut::Section::_morphology
  to null.
* added getter to mut::Section to retrieve owning morphology that will
  throw if the above pointer is null
* attempt to fix #161
  • Loading branch information
@mgeplf
mgeplf committed Mar 15, 2021
1 parent 75d944c commit a3ef7f6
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 32 deletions.
4 changes: 4 additions & 0 deletions include/morphio/mut/morphology.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,10 @@ class Morphology

std::map<uint32_t, uint32_t> _parent;
std::map<uint32_t, std::vector<std::shared_ptr<Section>>> _children;

private:
void eraseByValue(std::vector<std::shared_ptr<Section>>& vec,
const std::shared_ptr<Section> section);
};

inline const std::vector<std::shared_ptr<Section>>& Morphology::rootSections() const noexcept {
Expand Down
6 changes: 6 additions & 0 deletions include/morphio/mut/section.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,12 @@ class Section: public std::enable_shared_from_this<Section>
Section(Morphology*, unsigned int id, const morphio::Section& section);
Section(Morphology*, unsigned int id, const Section&);


/**
Getter for _morphology; checks the pointer is non-null, throws otherwise
**/
Morphology* getOwningMorphologyOrThrow() const;

Morphology* _morphology;
Property::PointLevel _pointProperties;
uint32_t _id;
Expand Down
9 changes: 7 additions & 2 deletions src/mut/morphology.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,14 +153,19 @@ Morphology::~Morphology() {
}
}

static void eraseByValue(std::vector<std::shared_ptr<Section>>& vec,
const std::shared_ptr<Section>& section) {
void Morphology::eraseByValue(std::vector<std::shared_ptr<Section>>& vec,
const std::shared_ptr<Section> section) {
if (section->_morphology == this) {
section->_morphology = nullptr;
section->_id = 0xffffffff;
}
vec.erase(std::remove(vec.begin(), vec.end(), section), vec.end());
}

void Morphology::deleteSection(std::shared_ptr<Section> section_, bool recursive) {
if (!section_)
return;

unsigned int id = section_->id();

if (recursive) {
Expand Down
75 changes: 45 additions & 30 deletions src/mut/section.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,32 @@ Section::Section(Morphology* morphology, unsigned int id_, const Section& sectio
, _id(id_)
, _sectionType(section_._sectionType) {}

Morphology* Section::getOwningMorphologyOrThrow() const {
if (!_morphology) {
throw std::runtime_error("Section does not belong to a morphology, impossible operation");
}
return _morphology;
}

const std::shared_ptr<Section>& Section::parent() const {
return _morphology->_sections.at(_morphology->_parent.at(id()));
const Morphology* morphology = getOwningMorphologyOrThrow();
return morphology->_sections.at(morphology->_parent.at(id()));
}

bool Section::isRoot() const {
const auto parentId = _morphology->_parent.find(id());
if (parentId != _morphology->_parent.end()) {
return _morphology->_sections.find(parentId->second) == _morphology->_sections.end();
const Morphology* morphology = getOwningMorphologyOrThrow();
const auto parentId = morphology->_parent.find(id());
if (parentId != morphology->_parent.end()) {
return morphology->_sections.find(parentId->second) == morphology->_sections.end();
}
return true;
}

const std::vector<std::shared_ptr<Section>>& Section::children() const {
const auto it = _morphology->_children.find(id());
if (it == _morphology->_children.end()) {
const Morphology* morphology = getOwningMorphologyOrThrow();

const auto it = morphology->_children.find(id());
if (it == morphology->_children.end()) {
static std::vector<std::shared_ptr<Section>> empty;
return empty;
}
Expand Down Expand Up @@ -91,26 +102,28 @@ std::ostream& operator<<(std::ostream& os, const std::shared_ptr<Section>& secti

std::shared_ptr<Section> Section::appendSection(std::shared_ptr<Section> original_section,
bool recursive) {
Morphology* morphology = getOwningMorphologyOrThrow();

const std::shared_ptr<Section> ptr(
new Section(_morphology, _morphology->_counter, *original_section));
new Section(morphology, morphology->_counter, *original_section));
unsigned int parentId = id();
uint32_t childId = _morphology->_register(ptr);
auto& _sections = _morphology->_sections;
uint32_t childId = morphology->_register(ptr);
auto& _sections = morphology->_sections;

bool emptySection = _emptySection(_sections[childId]);
if (emptySection)
printError(Warning::APPENDING_EMPTY_SECTION,
_morphology->_err.WARNING_APPENDING_EMPTY_SECTION(_sections[childId]));
morphology->_err.WARNING_APPENDING_EMPTY_SECTION(_sections[childId]));

if (!ErrorMessages::isIgnored(Warning::WRONG_DUPLICATE) && !emptySection &&
!_checkDuplicatePoint(_sections[parentId], _sections[childId])) {
printError(Warning::WRONG_DUPLICATE,
_morphology->_err.WARNING_WRONG_DUPLICATE(_sections[childId],
_sections.at(parentId)));
morphology->_err.WARNING_WRONG_DUPLICATE(_sections[childId],
_sections.at(parentId)));
}

_morphology->_parent[childId] = parentId;
_morphology->_children[parentId].push_back(ptr);
morphology->_parent[childId] = parentId;
morphology->_children[parentId].push_back(ptr);

// Careful not to use a reference here or you will face ref invalidation problem with vector
// resize The argument `original_section` of this function could be a reference to the
Expand All @@ -126,24 +139,25 @@ std::shared_ptr<Section> Section::appendSection(std::shared_ptr<Section> origina
}

std::shared_ptr<Section> Section::appendSection(const morphio::Section& section, bool recursive) {
const std::shared_ptr<Section> ptr(new Section(_morphology, _morphology->_counter, section));
Morphology* morphology = getOwningMorphologyOrThrow();
const std::shared_ptr<Section> ptr(new Section(morphology, morphology->_counter, section));
unsigned int parentId = id();
uint32_t childId = _morphology->_register(ptr);
auto& _sections = _morphology->_sections;
uint32_t childId = morphology->_register(ptr);
auto& _sections = morphology->_sections;

bool emptySection = _emptySection(_sections[childId]);
if (emptySection)
printError(Warning::APPENDING_EMPTY_SECTION,
_morphology->_err.WARNING_APPENDING_EMPTY_SECTION(_sections[childId]));
morphology->_err.WARNING_APPENDING_EMPTY_SECTION(_sections[childId]));

if (!ErrorMessages::isIgnored(Warning::WRONG_DUPLICATE) && !emptySection &&
!_checkDuplicatePoint(_sections[parentId], _sections[childId]))
printError(Warning::WRONG_DUPLICATE,
_morphology->_err.WARNING_WRONG_DUPLICATE(_sections[childId],
_sections.at(parentId)));
morphology->_err.WARNING_WRONG_DUPLICATE(_sections[childId],
_sections.at(parentId)));

_morphology->_parent[childId] = parentId;
_morphology->_children[parentId].push_back(ptr);
morphology->_parent[childId] = parentId;
morphology->_children[parentId].push_back(ptr);

if (recursive) {
for (auto child : section.children()) {
Expand All @@ -156,33 +170,34 @@ std::shared_ptr<Section> Section::appendSection(const morphio::Section& section,

std::shared_ptr<Section> Section::appendSection(const Property::PointLevel& pointProperties,
SectionType sectionType) {
Morphology* morphology = getOwningMorphologyOrThrow();
unsigned int parentId = id();

auto& _sections = _morphology->_sections;
auto& _sections = morphology->_sections;
if (sectionType == SectionType::SECTION_UNDEFINED)
sectionType = type();

if (sectionType == SECTION_SOMA)
throw morphio::SectionBuilderError("Cannot create section with type soma");

std::shared_ptr<Section> ptr(
new Section(_morphology, _morphology->_counter, sectionType, pointProperties));
new Section(morphology, morphology->_counter, sectionType, pointProperties));

uint32_t childId = _morphology->_register(ptr);
uint32_t childId = morphology->_register(ptr);

bool emptySection = _emptySection(_sections[childId]);
if (emptySection)
printError(Warning::APPENDING_EMPTY_SECTION,
_morphology->_err.WARNING_APPENDING_EMPTY_SECTION(_sections[childId]));
morphology->_err.WARNING_APPENDING_EMPTY_SECTION(_sections[childId]));

if (!ErrorMessages::isIgnored(Warning::WRONG_DUPLICATE) && !emptySection &&
!_checkDuplicatePoint(_sections[parentId], _sections[childId]))
printError(Warning::WRONG_DUPLICATE,
_morphology->_err.WARNING_WRONG_DUPLICATE(_sections[childId],
_sections[parentId]));
morphology->_err.WARNING_WRONG_DUPLICATE(_sections[childId],
_sections[parentId]));

_morphology->_parent[childId] = parentId;
_morphology->_children[parentId].push_back(ptr);
morphology->_parent[childId] = parentId;
morphology->_children[parentId].push_back(ptr);
return ptr;
}

Expand Down
43 changes: 43 additions & 0 deletions tests/test_5_mut.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

_path = os.path.join(os.path.dirname(os.path.abspath(__file__)), "data")

DATA_DIR = Path(__file__).parent / 'data'
SIMPLE = Morphology(os.path.join(_path, "simple.swc"))


Expand Down Expand Up @@ -509,3 +510,45 @@ def test_glia_round_trip():
g.write(filename)
g2 = GlialCell(filename)
assert_equal(len(g.sections), len(g2.sections))

def _get_section():
m = Morphology(DATA_DIR / 'simple.swc')
s = m.root_sections[0]
return s

def test_lifetime_1():
'''Attempting to access topological information after a Morphology has been destroyed should
raise'''
m = Morphology(DATA_DIR / 'simple.swc')
s = m.root_sections[0]
del m # ~mut.Morphology() called
with assert_raises(RuntimeError) as obj:
s.children

def test_lifetime_2():
'''Attempting to access topological information after a Morphology has been destroyed should
raise'''
section = _get_section()
assert_array_equal(section.points,
np.array([[0., 0., 0.],
[0., 5., 0.]], dtype=np.float32))

with assert_raises(RuntimeError) as obj:
section.children

def test_lifetime_3():
'''Copy a section from a destroyed morphology should work because it does not use any
topological information'''
section = _get_section()

# Proof that the morphology has really been destroyed
with assert_raises(RuntimeError) as obj:
section.children

morph = Morphology()
morph.append_root_section(section)
del section
assert_equal(len(morph.root_sections), 1)
assert_array_equal(morph.root_sections[0].points,
np.array([[0., 0., 0.],
[0., 5., 0.]], dtype=np.float32))

0 comments on commit a3ef7f6

Please sign in to comment.