ehmry/genode
ehmry
/
genode
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

base: fix faults in rm_session de-constructor path 

First make the clients inaccessible and dissolve them from the entrypoint. If
this isn't the first step the clients may be obtained again between
the unlock and lock steps in the destructor.

Additionally the clients may be removed in between the unlock and call
sequence, which renders such client pointers dangling and causes spurious page
faults. Keep instead a lock as long as possible and when it is required to
release a lock, then the pointer to the objects must be revalidated.

Replace the dissolve function with a remove_client implementation as suggested
by #13, which avoids that the cpu_session may call dissolve with a dangling
pointer of a already removed rm_client object. Instead the pager must be
released explicitly.

Related to issue #549
Related to issue #394
Related to issue #13
This commit is contained in:
Alexander Boettcher 2013-01-10 10:04:18 +01:00 committed by Norman Feske
parent cecfbf2eb4
commit f02958b25f
3 changed files with 88 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
base/src/core/cpu_session_component.cc
View File

@ -35,11 +35,9 @@ void Cpu_thread_component::update_exception_sigh()
Thread_capability Cpu_session_component::create_thread(Name const &name,
addr_t utcb)
{
Lock::Guard thread_list_lock_guard(_thread_list_lock);
Lock::Guard slab_lock_guard(_thread_alloc_lock);
Cpu_thread_component *thread = 0;
try {
Lock::Guard slab_lock_guard(_thread_alloc_lock);
thread = new(&_thread_alloc) Cpu_thread_component(name.string(),
_priority, utcb,
_default_exception_handler);
@ -47,35 +45,30 @@ Thread_capability Cpu_session_component::create_thread(Name const &name,
throw Out_of_metadata();
}
Lock::Guard thread_list_lock_guard(_thread_list_lock);
_thread_list.insert(thread);
return _thread_ep->manage(thread);
}
void Cpu_session_component::_unsynchronized_kill_thread(Cpu_thread_component *thread)
{
Lock::Guard lock_guard(_thread_alloc_lock);
_thread_ep->dissolve(thread);
_thread_list.remove(thread);
/* If the thread is associated with a rm_session dissolve it */
Rm_client *rc = dynamic_cast<Rm_client*>(thread->platform_thread()->pager());
if (rc)
rc->member_rm_session()->dissolve(rc);
Lock::Guard lock_guard(_thread_alloc_lock);
destroy(&_thread_alloc, thread);
}
void Cpu_session_component::kill_thread(Thread_capability thread_cap)
{
Lock::Guard lock_guard(_thread_list_lock);
Cpu_thread_component * thread =
dynamic_cast<Cpu_thread_component *>(_thread_ep->lookup_and_lock(thread_cap));
if (!thread) return;
Lock::Guard lock_guard(_thread_list_lock);
_unsynchronized_kill_thread(thread);
}

10
base/src/core/include/rm_session_component.h
View File

@ -135,7 +135,7 @@ namespace Genode {
* This function must be called when destructing region-manager
* sessions to prevent dangling pointers in '_faulters' lists.
*/
void dissolve_from_faulting_rm_session();
void dissolve_from_faulting_rm_session(Rm_session_component *);
/**
* Return true if page fault occurred in specified address range
@ -322,7 +322,7 @@ namespace Genode {
/**
* Dissolve faulter from region-manager session
*/
void discard_faulter(Rm_faulter *faulter);
void discard_faulter(Rm_faulter *faulter, bool do_lock);
List<Rm_client> *clients() { return &_clients; }
@ -336,11 +336,6 @@ namespace Genode {
*/
void upgrade_ram_quota(size_t ram_quota) { _md_alloc.upgrade(ram_quota); }
/**
* Dissolves client from region-manager session
*/
void dissolve(Rm_client *cl);
/**************************************
** Region manager session interface **
@ -349,6 +344,7 @@ namespace Genode {
Local_addr attach (Dataspace_capability, size_t, off_t, bool, Local_addr, bool);
void detach (Local_addr);
Pager_capability add_client (Thread_capability);
void remove_client (Pager_capability);
void fault_handler (Signal_context_capability handler);
State state ();
Dataspace_capability dataspace () { return _ds_cap; }

135
base/src/core/rm_session_component.cc
View File

@ -290,13 +290,13 @@ void Rm_faulter::fault(Rm_session_component *faulting_rm_session,
}
void Rm_faulter::dissolve_from_faulting_rm_session()
void Rm_faulter::dissolve_from_faulting_rm_session(Rm_session_component * caller)
{
/* serialize access */
Lock::Guard lock_guard(_lock);
if (_faulting_rm_session)
_faulting_rm_session->discard_faulter(this);
_faulting_rm_session->discard_faulter(this, _faulting_rm_session != caller);
_faulting_rm_session = 0;
}
@ -588,6 +588,39 @@ Pager_capability Rm_session_component::add_client(Thread_capability thread)
}
void Rm_session_component::remove_client(Pager_capability pager_cap)
{
Rm_client * cl = dynamic_cast<Rm_client *>(_pager_ep->lookup_and_lock(pager_cap));
if (!cl) return;
/*
* Rm_client is derived from Pager_object. If the Pager_object is also
* derived from Thread_base then the Rm_client object must be
* destructed without holding the rm_session_object lock. The native
* platform specific Thread_base implementation has to take care that
* all in-flight page handling requests are finished before
* destruction. (Either by waiting until the end of or by
* <deadlock free> cancellation of the last in-flight request.
* This operation can also require taking the rm_session_object lock.
*/
{
Lock::Guard lock_guard(_lock);
_clients.remove(cl);
}
/* call platform specific dissolve routines */
_pager_ep->dissolve(cl);
{
Lock::Guard lock_guard(_lock);
cl->dissolve_from_faulting_rm_session(this);
}
destroy(&_client_slab, cl);
}
bool Rm_session_component::reverse_lookup(addr_t dst_base,
Fault_area *dst_fault_area,
Dataspace_component **src_dataspace,
@ -650,7 +683,7 @@ void Rm_session_component::fault(Rm_faulter *faulter, addr_t pf_addr,
/* serialize access */
Lock::Guard lock_guard(_lock);
/* remeber fault state in faulting thread */
/* remember fault state in faulting thread */
faulter->fault(this, Rm_session::State(pf_type, pf_addr));
/* enqueue faulter */
@ -661,12 +694,13 @@ void Rm_session_component::fault(Rm_faulter *faulter, addr_t pf_addr,
}
void Rm_session_component::discard_faulter(Rm_faulter *faulter)
void Rm_session_component::discard_faulter(Rm_faulter *faulter, bool do_lock)
{
/* serialize access */
Lock::Guard lock_guard(_lock);
_faulters.remove(faulter);
if (do_lock) {
Lock::Guard lock_guard(_lock);
_faulters.remove(faulter);
} else
_faulters.remove(faulter);
}
@ -692,35 +726,6 @@ Rm_session::State Rm_session_component::state()
return faulter->fault_state();
}
void Rm_session_component::dissolve(Rm_client *cl)
{
{
Lock::Guard lock_guard(_lock);
_pager_ep->dissolve(cl);
_clients.remove(cl);
}
/*
* Rm_client is derived from Pager_object. If the Pager_object is also
* derived from Thread_base then the Rm_client object must be
* destructed without holding the rm_session_object lock. The native
* platform specific Thread_base implementation has to take care that
* all in-flight page handling requests are finished before
* destruction. (Either by waiting until the end of or by
* <deadlock free> cancellation of the last in-flight request.
* This operation can also require taking the rm_session_object lock.
*
* Since _client_slab insertion/deletion also must be performed
* synchronized but can't be protected by the rm_session_object lock
* because of the described potential dead_lock situation, we have
* to use a synchronized allocator object to perform insertion and
* deletion of Rm_clients.
*/
destroy(&_client_slab, cl);
}
static Dataspace_capability _type_deduction_helper(Dataspace_capability cap) {
return cap; }
@ -746,20 +751,48 @@ Rm_session_component::Rm_session_component(Rpc_entrypoint *ds_ep,
Rm_session_component::~Rm_session_component()
{
_lock.lock();
/* dissolve all clients from pager entrypoint */
Rm_client *cl;
do {
{
Lock::Guard lock_guard(_lock);
cl = _clients.first();
if (!cl) break;
_clients.remove(cl);
}
/* call platform specific dissolve routines */
_pager_ep->dissolve(cl);
} while (cl);
/* detach all regions */
Rm_region_ref *ref;
do {
void * local_addr;
{
Lock::Guard lock_guard(_lock);
ref = _ref_slab.first_object();
if (!ref) break;
local_addr = reinterpret_cast<void *>(ref->region()->base());
}
detach(local_addr);
} while (ref);
/* revoke dataspace representation */
_ds_ep->dissolve(&_ds);
/* remove all faulters with pending page faults at this rm session */
while (Rm_faulter *faulter = _faulters.head()) {
_lock.unlock();
faulter->dissolve_from_faulting_rm_session();
_lock.lock();
}
/* serialize access */
_lock.lock();
/* remove all clients */
/* remove all faulters with pending page faults at this rm session */
while (Rm_faulter *faulter = _faulters.head())
faulter->dissolve_from_faulting_rm_session(this);
/* remove all clients, invalidate rm_client pointers in cpu_thread objects */
while (Rm_client *cl = _client_slab.raw()->first_object()) {
cl->dissolve_from_faulting_rm_session(this);
Thread_capability thread_cap = cl->thread_cap();
if (thread_cap.valid())
/* invalidate thread cap in rm_client object */
@ -767,24 +800,16 @@ Rm_session_component::~Rm_session_component()
_lock.unlock();
cl->dissolve_from_faulting_rm_session();
this->dissolve(cl);
{
/* lookup thread and reset pager pointer */
Object_pool<Cpu_thread_component>::Guard
cpu_thread(_thread_ep->lookup_and_lock(thread_cap));
if (cpu_thread)
if (cpu_thread && (cpu_thread->platform_thread()->pager() == cl))
cpu_thread->platform_thread()->pager(0);
}
_lock.lock();
}
destroy(&_client_slab, cl);
/* detach all regions */
while (Rm_region_ref *r = _ref_slab.first_object()) {
_lock.unlock();
detach((void *)r->region()->base());
_lock.lock();
}