Changeset 97b8ca9 in mainline

Timestamp:

2016-09-11T08:44:04Z (8 years ago)

Author:

Jakub Jermar <jakub@…>

Branches:

lfn, master, serial, ticket/834-toolchain-update, topic/msim-upgrade, topic/simplify-dev-export

Children:

560b81c

Parents:

0b00599

Message:

Fix race between ipc_answerbox_slam_phones() and ipc_cleanup()

We modify _ipc_call() to support sending of pre-forgotten calls. This is
handy in ipc_answerbox_slam_phones() when after slamming a client's
phone, we may no longer be sure of the client's continued existence.

A pre-forgotten call is delivered to the callee in an already forgotten
state, so that the caller is no longer expected to wait for it nor it is
expected to be still around. The caller is held alive for the necessary
time, but can disappear as soon as we release it.

File:

• kernel/generic/src/ipc/ipc.c (modified) (9 diffs)

kernel/generic/src/ipc/ipc.c

@@ -328 +328 @@
 }
 
-static void _ipc_call_actions_internal(phone_t *phone, call_t *call)
+static void _ipc_call_actions_internal(phone_t *phone, call_t *call,
+    bool preforget)
 {
         task_t *caller = phone->caller;
 
-        atomic_inc(&phone->active_calls);
         call->caller_phone = phone;
-        call->sender = caller;
-
-        call->active = true;
-        spinlock_lock(&caller->active_calls_lock);
-        list_append(&call->ta_link, &caller->active_calls);
-        spinlock_unlock(&caller->active_calls_lock);
+
+        if (preforget) {
+                call->forget = true;
+        } else {
+                atomic_inc(&phone->active_calls);
+                call->sender = caller;
+                call->active = true;
+                spinlock_lock(&caller->active_calls_lock);
+                list_append(&call->ta_link, &caller->active_calls);
+                spinlock_unlock(&caller->active_calls_lock);
+        }
 
         call->data.phone = phone;
@@ -357 +362 @@
 void ipc_backsend_err(phone_t *phone, call_t *call, sysarg_t err)
 {
-        _ipc_call_actions_internal(phone, call);
+        _ipc_call_actions_internal(phone, call, false);
         IPC_SET_RETVAL(call->data, err);
         _ipc_answer_free_call(call, false);
@@ -364 +369 @@
 /** Unsafe unchecking version of ipc_call.
  *
- * @param phone Phone structure the call comes from.
- * @param box   Destination answerbox structure.
- * @param call  Call structure with request.
- *
- */
-static void _ipc_call(phone_t *phone, answerbox_t *box, call_t *call)
+ * @param phone     Phone structure the call comes from.
+ * @param box       Destination answerbox structure.
+ * @param call      Call structure with request.
+ * @param preforget If true, the call will be delivered already forgotten.
+ *
+ */
+static void _ipc_call(phone_t *phone, answerbox_t *box, call_t *call,
+    bool preforget)
 {
         task_t *caller = phone->caller;
@@ -379 +386 @@
         
         if (!(call->flags & IPC_CALL_FORWARDED))
-                _ipc_call_actions_internal(phone, call);
+                _ipc_call_actions_internal(phone, call, preforget);
         
         irq_spinlock_lock(&box->lock, true);
@@ -413 +420 @@
         
         answerbox_t *box = phone->callee;
-        _ipc_call(phone, box, call);
+        _ipc_call(phone, box, call, false);
         
         mutex_unlock(&phone->lock);
@@ -451 +458 @@
                 call->request_method = IPC_M_PHONE_HUNGUP;
                 call->flags |= IPC_CALL_DISCARD_ANSWER;
-                _ipc_call(phone, box, call);
+                _ipc_call(phone, box, call, false);
         }
         
@@ -610 +617 @@
         phone_t *phone;
         DEADLOCK_PROBE_INIT(p_phonelck);
-        
-        call_t *call = notify_box ? ipc_call_alloc(0) : NULL;
         
         /* Disconnect all phones connected to our answerbox */
@@ -632 +637 @@
                 
                 if (notify_box) {
+                        task_hold(phone->caller);
                         mutex_unlock(&phone->lock);
                         irq_spinlock_unlock(&box->lock, true);
 
-                        // FIXME: phone can become deallocated at any time now
-
                         /*
-                         * Send one message to the answerbox for each
-                         * phone. Used to make sure the kbox thread
-                         * wakes up after the last phone has been
-                         * disconnected.
+                         * Send one call to the answerbox for each phone.
+                         * Used to make sure the kbox thread wakes up after
+                         * the last phone has been disconnected. The call is
+                         * forgotten upon sending, so the "caller" may cease
+                         * to exist as soon as we release it.
                          */
+                        call_t *call = ipc_call_alloc(0);
                         IPC_SET_IMETHOD(call->data, IPC_M_PHONE_HUNGUP);
                         call->request_method = IPC_M_PHONE_HUNGUP;
                         call->flags |= IPC_CALL_DISCARD_ANSWER;
-                        _ipc_call(phone, box, call);
-                        
-                        /* Allocate another call in advance */
-                        call = ipc_call_alloc(0);
+                        _ipc_call(phone, box, call, true);
+
+                        task_release(phone->caller);
                         
                         /* Must start again */
@@ -659 +664 @@
         
         irq_spinlock_unlock(&box->lock, true);
-        
-        /* Free unused call */
-        if (call)
-                ipc_call_free(call);
 }