diff --git a/repos/base/include/base/registry.h b/repos/base/include/base/registry.h index 18d7d8f179..75af643adc 100644 --- a/repos/base/include/base/registry.h +++ b/repos/base/include/base/registry.h @@ -29,7 +29,12 @@ class Genode::Registry_base { private: - enum Keep { KEEP, DISCARD }; + struct Notify { + enum Keep { KEEP, DISCARD } keep; + void * thread; + + Notify(Keep k, void *t) : keep(k), thread(t) { } + }; protected: @@ -49,7 +54,7 @@ class Genode::Registry_base /* * Assigned by 'Registry::_for_each' */ - Keep *_keep_ptr = nullptr; + Notify *_notify_ptr = nullptr; protected: @@ -77,7 +82,7 @@ class Genode::Registry_base void _insert(Element &); void _remove(Element &); - Element *_processed(Keep, List &, Element &, Element *); + Element *_processed(Notify &, List &, Element &, Element *); protected: diff --git a/repos/base/src/lib/base/registry.cc b/repos/base/src/lib/base/registry.cc index 97ca6cb9a5..07ef5a2b44 100644 --- a/repos/base/src/lib/base/registry.cc +++ b/repos/base/src/lib/base/registry.cc @@ -12,6 +12,7 @@ */ #include +#include using namespace Genode; @@ -28,15 +29,26 @@ Registry_base::Element::~Element() { { Lock::Guard guard(_lock); - if (_keep_ptr && _registry._curr == this) { + if (_notify_ptr && _registry._curr == this) { /* * The destructor is called from the functor of a * 'Registry::for_each' loop with the element temporarily dequeued. * We flag the element to be not re-inserted into the list. */ - *_keep_ptr = DISCARD; - return; + _notify_ptr->keep = Notify::Keep::DISCARD; + + /* done if and only if running in the context of same thread */ + if (Thread::myself() == _notify_ptr->thread) + return; + + /* + * We synchronize on the _lock of the _registry, by invoking + * the _remove method below. This ensures that the object leaves + * the destructor not before the registry lost the pointer to this + * object. The actual removal attempt will be ignored by the list + * implementation, since the current object was removed already. + */ } } _registry._remove(*this); @@ -59,31 +71,31 @@ void Registry_base::_remove(Element &element) } -Registry_base::Element *Registry_base::_processed(Keep const keep, +Registry_base::Element *Registry_base::_processed(Notify ¬ify, List &processed, Element &e, Element *at) { _curr = nullptr; /* if 'e' was dropped from the list, keep the current re-insert position */ - if (keep == DISCARD) + if (notify.keep == Notify::Keep::DISCARD) return at; /* make sure that the critical section of '~Element' is completed */ Lock::Guard guard(e._lock); + /* here we know that 'e' still exists */ + e._notify_ptr = nullptr; + /* * If '~Element' was preempted between the condition check and the * assignment of keep = DISCARD, the above check would miss the DISCARD * flag. Now, with the acquired lock, we know that the 'keep' value is * up to date. */ - if (keep == DISCARD) + if (notify.keep == Notify::Keep::DISCARD) return at; - /* here we know that 'e' still exists */ - e._keep_ptr = nullptr; - /* insert 'e' into the list of processed elements */ processed.insert(&e, at); @@ -103,12 +115,12 @@ void Registry_base::_for_each(Untyped_functor &functor) while (Element *e = _elements.first()) { - Keep keep = KEEP; + Notify notify(Notify::Keep::KEEP, Thread::myself()); { /* tell the element where to report its status */ Lock::Guard guard(e->_lock); _curr = e; - e->_keep_ptr = &keep; + e->_notify_ptr = ¬ify; } /* @@ -122,9 +134,9 @@ void Registry_base::_for_each(Untyped_functor &functor) try { functor.call(e->_obj); } /* propagate exceptions while keeping registry consistent */ - catch (...) { at = _processed(keep, processed, *e, at); throw; } + catch (...) { at = _processed(notify, processed, *e, at); throw; } - at = _processed(keep, processed, *e, at); + at = _processed(notify, processed, *e, at); } /* use list of processed elements as '_elements' list */