[LibOS] Trigger callbacks only after list traversal in Async Helper

In Async Helper thread, async_list is protected by async_list_lock.
Previously, this thread unlocked async_list_lock on each detected
triggered entry to perform a callback during traversal of async_list
(unlocking is needed because it is unknown how much time the callback
takes to execute). However, once we unlock, the list may become
unstable (install_async_event may delete/append an entry). Then, next
FOR_EACH_LIST_SAFE list entry may be freed while trigering callback,
and the next iteration may try to dereference this invalid pointer.
This commit alleviates this scenario by creating a temporal list of
triggered entries and performing all callbacks after list traversal.
This commit is contained in:
Isaku Yamahata
2019-12-03 05:15:47 -08:00
committed by Dmitrii Kuvaiskii
parent ce7e029d9b
commit c40b333728
+24 -4
View File
@@ -193,6 +193,10 @@ static void shim_async_helper(void * arg) {
break;
}
LISTP_TYPE(async_event) triggered;
INIT_LISTP(&triggered);
again:;
/* Iterate through all async IO events and alarm/timer events to:
* - call callbacks for all triggered events, and
* - repopulate object_list with async IO events (if any), and
@@ -207,16 +211,22 @@ static void shim_async_helper(void * arg) {
if (polled && tmp->object == polled) {
debug("Async IO event triggered at %lu\n", now);
unlock(&async_helper_lock);
/* FIXME: potential race condition when
* ioctl(FIOASYNC, off) and cleanup on fd-close are
* correctly implemented. tmp can be freed at the same
* time. */
tmp->callback(tmp->caller, tmp->arg);
/* async_list may be changed because async_helper_lock is
* released; list traverse cannot be continued. */
polled = NULL;
lock(&async_helper_lock);
goto again;
} else if (tmp->expire_time && tmp->expire_time <= now) {
debug("Async alarm/timer triggered at %lu (expired at %lu)\n",
now, tmp->expire_time);
LISTP_DEL(tmp, &async_list, list);
unlock(&async_helper_lock);
tmp->callback(tmp->caller, tmp->arg);
free(tmp);
lock(&async_helper_lock);
LISTP_ADD_TAIL(tmp, &triggered, list);
continue;
}
@@ -242,6 +252,16 @@ static void shim_async_helper(void * arg) {
}
}
if (!LISTP_EMPTY(&triggered)) {
unlock(&async_helper_lock);
LISTP_FOR_EACH_ENTRY_SAFE(tmp, n, &triggered, list) {
LISTP_DEL(tmp, &triggered, list);
tmp->callback(tmp->caller, tmp->arg);
free(tmp);
}
lock(&async_helper_lock);
}
uint64_t sleep_time;
if (next_expire_time) {
sleep_time = next_expire_time - now;