diff --git a/repos/base-codezero/src/core/platform_pd.cc b/repos/base-codezero/src/core/platform_pd.cc
index 89431ec5b7..33e1b11151 100644
--- a/repos/base-codezero/src/core/platform_pd.cc
+++ b/repos/base-codezero/src/core/platform_pd.cc
@@ -123,5 +123,8 @@ Platform_pd::Platform_pd(Allocator * md_alloc, char const *,
Platform_pd::~Platform_pd()
{
+ /* invalidate weak pointers to this object */
+ Address_space::lock_for_destruction();
+
PWRN("not yet implemented");
}
diff --git a/repos/base-fiasco/src/core/platform_pd.cc b/repos/base-fiasco/src/core/platform_pd.cc
index 4ce85e21cd..11cf50ffe2 100644
--- a/repos/base-fiasco/src/core/platform_pd.cc
+++ b/repos/base-fiasco/src/core/platform_pd.cc
@@ -258,6 +258,9 @@ Platform_pd::Platform_pd(Allocator * md_alloc, char const *,
Platform_pd::~Platform_pd()
{
+ /* invalidate weak pointers to this object */
+ Address_space::lock_for_destruction();
+
/* unbind all threads */
while (Platform_thread *t = _next_thread()) unbind_thread(t);
diff --git a/repos/base-foc/src/core/platform_pd.cc b/repos/base-foc/src/core/platform_pd.cc
index 52fdba1025..23c433b5fc 100644
--- a/repos/base-foc/src/core/platform_pd.cc
+++ b/repos/base-foc/src/core/platform_pd.cc
@@ -128,6 +128,9 @@ Platform_pd::Platform_pd()
Platform_pd::~Platform_pd()
{
+ /* invalidate weak pointers to this object */
+ Address_space::lock_for_destruction();
+
for (unsigned i = 0; i < THREAD_MAX; i++) {
if (_threads[i])
_threads[i]->unbind();
diff --git a/repos/base-nova/src/core/platform_pd.cc b/repos/base-nova/src/core/platform_pd.cc
index c5d1763a28..c9b31ce921 100644
--- a/repos/base-nova/src/core/platform_pd.cc
+++ b/repos/base-nova/src/core/platform_pd.cc
@@ -53,6 +53,9 @@ Platform_pd::Platform_pd(Allocator * md_alloc, char const *,
Platform_pd::~Platform_pd()
{
+ /* invalidate weak pointers to this object */
+ Address_space::lock_for_destruction();
+
if (_pd_sel == ~0UL) return;
/* Revoke and free cap, pd is gone */
diff --git a/repos/base-okl4/src/core/platform_pd.cc b/repos/base-okl4/src/core/platform_pd.cc
index 43ac47fef4..067dd3ab82 100644
--- a/repos/base-okl4/src/core/platform_pd.cc
+++ b/repos/base-okl4/src/core/platform_pd.cc
@@ -354,6 +354,9 @@ Platform_pd::Platform_pd(signed pd_id, bool create)
Platform_pd::~Platform_pd()
{
+ /* invalidate weak pointers to this object */
+ Address_space::lock_for_destruction();
+
/* unbind all threads */
while (Platform_thread *t = _next_thread()) unbind_thread(t);
diff --git a/repos/base-pistachio/src/core/platform_pd.cc b/repos/base-pistachio/src/core/platform_pd.cc
index 84c6a65cfc..048c00b9dc 100644
--- a/repos/base-pistachio/src/core/platform_pd.cc
+++ b/repos/base-pistachio/src/core/platform_pd.cc
@@ -360,6 +360,9 @@ Platform_pd::Platform_pd(Allocator * md_alloc, char const *,
Platform_pd::~Platform_pd()
{
+ /* invalidate weak pointers to this object */
+ Address_space::lock_for_destruction();
+
PT_DBG("Destroying all threads of pd %p", this);
/* unbind all threads */
diff --git a/repos/base-sel4/src/core/platform_pd.cc b/repos/base-sel4/src/core/platform_pd.cc
index 801b58a15e..1ec4f5573f 100644
--- a/repos/base-sel4/src/core/platform_pd.cc
+++ b/repos/base-sel4/src/core/platform_pd.cc
@@ -170,6 +170,9 @@ Platform_pd::Platform_pd(Allocator * md_alloc, char const *,
Platform_pd::~Platform_pd()
{
+ /* invalidate weak pointers to this object */
+ Address_space::lock_for_destruction();
+
platform_specific()->free_core_sel(_vm_cnode_sel);
platform_specific()->free_core_sel(_vm_pad_cnode_sel);
platform_specific()->free_core_sel(_cspace_cnode_sel);
diff --git a/repos/base/include/base/weak_ptr.h b/repos/base/include/base/weak_ptr.h
index 17289171d9..79d60c2d27 100644
--- a/repos/base/include/base/weak_ptr.h
+++ b/repos/base/include/base/weak_ptr.h
@@ -15,6 +15,7 @@
#define _INCLUDE__BASE__WEAK_PTR_H_
#include
+#include
#include
namespace Genode {
@@ -43,21 +44,18 @@ class Genode::Weak_ptr_base : public Genode::List::Element
Lock mutable _lock;
Weak_object_base *_obj;
- bool _valid; /* true if '_obj' points to an
- existing object */
+
+ /*
+ * This lock is used to synchronize destruction of a weak pointer
+ * and its corresponding weak object that happen simultanously
+ */
+ Lock mutable _destruct_lock { Lock::LOCKED };
inline void _adopt(Weak_object_base *obj);
inline void _disassociate();
protected:
- /**
- * Return pointer to object if it exists, or 0 if object vanished
- *
- * \noapi
- */
- Weak_object_base *obj() const { return _valid ? _obj: 0; }
-
explicit inline Weak_ptr_base(Weak_object_base *obj);
public:
@@ -79,6 +77,13 @@ class Genode::Weak_ptr_base : public Genode::List::Element
*/
inline bool operator == (Weak_ptr_base const &other) const;
+ /**
+ * Return pointer to object if it exists, or 0 if object vanished
+ *
+ * \noapi
+ */
+ Weak_object_base *obj() const { return _obj; }
+
/**
* Inspection hook for unit test
*
@@ -105,19 +110,16 @@ class Genode::Weak_object_base
List _list;
/**
- * Lock used to defer the destruction of an object derived from
- * 'Weak_object_base'
+ * Buffers dequeued weak pointer that get invalidated currently
*/
- Lock _destruct_lock;
-
- protected:
+ Weak_ptr_base *_ptr_in_destruction = nullptr;
/**
- * Destructor
- *
- * \noapi
+ * Lock to synchronize access to object
*/
- inline ~Weak_object_base();
+ Lock _lock;
+
+ protected:
/**
* To be called from 'Weak_object' only
@@ -129,13 +131,84 @@ class Genode::Weak_object_base
public:
+ /**
+ * This exception signals a weak pointer that the object
+ * is in destruction progress
+ */
+ class In_destruction : Exception {};
+
+ ~Weak_object_base()
+ {
+ if (_list.first())
+ PERR("Weak object %p not destructed properly "
+ "there are still dangling pointers to it", this);
+ }
+
+ void disassociate(Weak_ptr_base *ptr)
+ {
+ if (!ptr) return;
+
+ {
+ Lock::Guard guard(_list_lock);
+
+ /*
+ * If the weak pointer that tries to disassociate is currently
+ * removed to invalidate it by the weak object's destructor,
+ * signal that fact to the pointer, so it can free it's lock,
+ * and block until invalidation is finished.
+ */
+ if (_ptr_in_destruction == ptr)
+ throw In_destruction();
+
+ _list.remove(ptr);
+ }
+ }
+
/**
* Mark object as safe to be destructed
*
* This method must be called by the destructor of a weak object to
* defer the destruction until no 'Locked_ptr' is held to the object.
*/
- void lock_for_destruction() { _destruct_lock.lock(); }
+ void lock_for_destruction()
+ {
+ /*
+ * Loop through the list of weak pointers and invalidate them
+ */
+ while (true) {
+
+ /*
+ * To prevent dead-locks we always have to hold
+ * the order of lock access, therefore we first
+ * dequeue one weak pointer and free the list lock again
+ */
+ {
+ Lock::Guard guard(_list_lock);
+ _ptr_in_destruction = _list.first();
+
+ /* if the list is empty we're done */
+ if (!_ptr_in_destruction) break;
+ _list.remove(_ptr_in_destruction);
+ }
+
+ {
+ Lock::Guard guard(_ptr_in_destruction->_lock);
+ _ptr_in_destruction->_obj = nullptr;
+
+ /*
+ * unblock a weak pointer that tried to disassociate
+ * in the meantime
+ */
+ _ptr_in_destruction->_destruct_lock.unlock();
+ }
+ }
+
+ /*
+ * synchronize with locked pointers that already aquired
+ * the lock before the corresponding weak pointer got invalidated
+ */
+ _lock.lock();
+ }
/**
* Inspection hook for unit test
@@ -249,7 +322,7 @@ struct Genode::Locked_ptr : Genode::Locked_ptr_base
* Only if valid, the locked pointer can be de-referenced. Otherwise,
* the attempt will result in a null-pointer access.
*/
- bool is_valid() const { return curr != 0; }
+ bool is_valid() const { return curr != nullptr; }
};
@@ -263,41 +336,24 @@ void Genode::Weak_ptr_base::_adopt(Genode::Weak_object_base *obj)
return;
_obj = obj;
- _valid = true;
- Lock::Guard guard(_obj->_list_lock);
- _obj->_list.insert(this);
+ {
+ Lock::Guard guard(_obj->_list_lock);
+ _obj->_list.insert(this);
+ }
}
void Genode::Weak_ptr_base::_disassociate()
{
/* defer destruction of object */
- {
+ try {
Lock::Guard guard(_lock);
- if (!_valid)
- return;
-
- _obj->_destruct_lock.lock();
+ if (_obj) _obj->disassociate(this);
+ } catch(Weak_object_base::In_destruction&) {
+ _destruct_lock.lock();
}
-
- /*
- * Disassociate reference from object
- *
- * Because we hold the '_destruct_lock', we are safe to do
- * the list operation. However, after we have released the
- * 'Weak_ptr_base::_lock', the object may have invalidated
- * the reference. So we must check for validity again.
- */
- {
- Lock::Guard guard(_obj->_list_lock);
- if (_valid)
- _obj->_list.remove(this);
- }
-
- /* release object */
- _obj->_destruct_lock.unlock();
}
@@ -307,7 +363,7 @@ Genode::Weak_ptr_base::Weak_ptr_base(Genode::Weak_object_base *obj)
}
-Genode::Weak_ptr_base::Weak_ptr_base() : _obj(0), _valid(false) { }
+Genode::Weak_ptr_base::Weak_ptr_base() : _obj(nullptr) { }
void Genode::Weak_ptr_base::operator = (Weak_ptr_base const &other)
@@ -316,9 +372,11 @@ void Genode::Weak_ptr_base::operator = (Weak_ptr_base const &other)
if (&other == this)
return;
- Weak_object_base *obj = other.obj();
_disassociate();
- _adopt(obj);
+ {
+ Lock::Guard guard(other._lock);
+ _adopt(other._obj);
+ }
}
@@ -329,8 +387,7 @@ bool Genode::Weak_ptr_base::operator == (Weak_ptr_base const &other) const
Lock::Guard guard_this(_lock), guard_other(other._lock);
- return (!_valid && !other._valid)
- || (_valid && other._valid && _obj == other._obj);
+ return (_obj == other._obj);
}
@@ -348,39 +405,22 @@ Genode::Weak_ptr Genode::Weak_object_base::_weak_ptr()
}
-Genode::Weak_object_base::~Weak_object_base()
-{
- {
- Lock::Guard guard(_list_lock);
-
- Weak_ptr_base *curr = 0;
- while ((curr = _list.first())) {
-
- Lock::Guard guard(curr->_lock);
- curr->_valid = false;
- _list.remove(curr);
- }
- }
-}
-
-
Genode::Locked_ptr_base::Locked_ptr_base(Weak_ptr_base &weak_ptr)
-: curr(0)
+: curr(nullptr)
{
Lock::Guard guard(weak_ptr._lock);
- if (!weak_ptr._valid)
- return;
+ if (!weak_ptr.obj()) return;
- curr = weak_ptr._obj;
- curr->_destruct_lock.lock();
+ curr = weak_ptr.obj();
+ curr->_lock.lock();
}
Genode::Locked_ptr_base::~Locked_ptr_base()
{
if (curr)
- curr->_destruct_lock.unlock();
+ curr->_lock.unlock();
}
#endif /* _INCLUDE__BASE__WEAK_PTR_H_ */
diff --git a/repos/base/src/core/include/trace/source_registry.h b/repos/base/src/core/include/trace/source_registry.h
index d873546046..d503969fc6 100644
--- a/repos/base/src/core/include/trace/source_registry.h
+++ b/repos/base/src/core/include/trace/source_registry.h
@@ -84,6 +84,11 @@ class Genode::Trace::Source
_unique_id(_alloc_unique_id()), _info(info), _control(control)
{ }
+ ~Source()
+ {
+ /* invalidate weak pointers to this object */
+ lock_for_destruction();
+ }
/*************************************
** Interface used by TRACE service **
diff --git a/repos/os/src/server/nitpicker/view.h b/repos/os/src/server/nitpicker/view.h
index 4e3ee30373..9a0108c5a4 100644
--- a/repos/os/src/server/nitpicker/view.h
+++ b/repos/os/src/server/nitpicker/view.h
@@ -126,6 +126,9 @@ class View : public Same_buffer_list_elem,
virtual ~View()
{
+ /* invalidate weak pointers to this object */
+ lock_for_destruction();
+
/* break link to our parent */
if (_parent)
_parent->remove_child(*this);