improve killing processes/threads, test

+ Remove sys_kill() as we should be using a two-stage process where a
  thread is first orphaned by sys_detach(), and then the thread itself
  calls sys_exit() after it has done all necessary cleanup in init.

6 files changed, 38 insertions, 86 deletions

diff --git a/src/orphanage.c b/src/orphanage.c
index 526affc..797468d 100644
--- a/src/orphanage.c
+++ b/src/orphanage.c
@@ -29,16 +29,16 @@ void unorphanize(struct tcb *t)
 
 	/* attach to init process */
 	struct tcb *init = get_tcb(1);
-	reference_proc(init);
+	reference_thread(init);
+
+	free_stack(t);
+	reset_rpc_stack(t);
 
 	id_t old_rid = t->rid;
 	t->rid = 1;
 	t->pid = 1;
 	t->eid = 1;
 
-	free_stack(t);
-	reset_rpc_stack(t);
-
 	t->proc = init->proc;
 	use_vmem(t->proc.vmem);
 	alloc_stack(t);
diff --git a/src/tcb.c b/src/tcb.c
index 238d8e3..93ce502 100644
--- a/src/tcb.c
+++ b/src/tcb.c
@@ -120,7 +120,7 @@ stat_t alloc_stack(struct tcb *t)
 
 void free_stack(struct tcb *t)
 {
-	free_uvmem(t, t->thread_stack);
+	free_uvmem(get_proc(t), t->thread_stack);
 }
 
 struct tcb *create_thread(struct tcb *p)
@@ -180,7 +180,7 @@ struct tcb *create_thread(struct tcb *p)
 	}
 
 	setup_rpc_stack(t);
-	reference_proc(p);
+	reference_thread(p);
 
 	t->regs = (vm_t)t;
 
@@ -235,15 +235,9 @@ struct tcb *create_proc(struct tcb *p)
 static stat_t __destroy_thread_data(struct tcb *t)
 {
 	assert(t->refcount == 0);
+	assert(zombie(t));
 
 	/* remove ourselves from the thread pool */
-	/** @todo this should be at the top of the function, and be wrapped in
-	 * some kind of lock that checks that nobody reads the value while we're
-	 * setting it to zero. get_tcb() should accordingly increment the
-	 * reference count atomically. Also, an unget_tcb() is needed to
-	 * decrement the reference count I guess? if we didn't have the BKL that
-	 * is
-	 */
 	tcbs[t->tid] = 0;
 
 	/* forcefully free last struggling bits of memory, assuming we own the
@@ -259,14 +253,10 @@ static stat_t __destroy_thread_data(struct tcb *t)
 stat_t destroy_thread(struct tcb *t)
 {
 	assert(tcbs);
-	assert(!is_proc(t));
-
-	/* mark us as zombies */
-	set_bits(t->state, TCB_ZOMBIE);
-	t->rid = 0;
+	assert(t->tid != 1);
 
 	/* remove reference to root process */
-	unreference_proc(get_rproc(t));
+	unreference_thread(get_rproc(t));
 
 	free_stack(t);
 
@@ -282,12 +272,12 @@ stat_t destroy_thread(struct tcb *t)
 	 * let the handler check if the thread is still interested in the
 	 * interrupt */
 
-	/* someone still relies on us existing, don't actually free thread data
-	 * quite yet */
-	if (t->refcount)
-		return OK;
+	/* mark us as zombies */
+	set_bits(t->state, TCB_ZOMBIE);
+	t->rid = 0;
 
-	return __destroy_thread_data(t);
+	unreference_thread(t);
+	return OK;
 }
 
 stat_t destroy_proc(struct tcb *p)
@@ -295,15 +285,6 @@ stat_t destroy_proc(struct tcb *p)
 	assert(tcbs);
 	assert(is_proc(p));
 
-	/** @todo currently we don't care who else is in the address space when
-	 * we start freeing stuff, one fairly simple way to deal with this is to
-	 * just not care. A thread that tries to access some bit of freed memory
-	 * will cause a segfault (eventually at least), and we can just check in
-	 * the segfault handler if the thread has become orphaned.
-	 * Currently no segfault handler exists, though. */
-
-	set_bits(p->state, TCB_ZOMBIE);
-
 	/* clear all privately owned memory regions, keep shared ones alive for
 	 * now */
 	clear_uvmem(p);
@@ -313,25 +294,23 @@ stat_t destroy_proc(struct tcb *p)
 	return OK;
 }
 
-void reference_proc(struct tcb *p)
+void reference_thread(struct tcb *t)
 {
-	if (!p)
+	if (!t)
 		return;
 
-	assert(is_proc(p));
-	p->refcount++;
+	t->refcount++;
 }
 
-void unreference_proc(struct tcb *p)
+void unreference_thread(struct tcb *t)
 {
-	if (!p)
+	if (!t)
 		return;
 
-	assert(is_proc(p));
-	p->refcount--;
-	if (zombie(p) && p->refcount == 0) {
-		dbg("thread %ld is completely destroyed\n", (long)p->tid);
-		__destroy_thread_data(p);
+	t->refcount--;
+	if (t->refcount == 0) {
+		info("thread %ld is completely destroyed\n", (long)t->tid);
+		__destroy_thread_data(t);
 	}
 }
 
diff --git a/src/uapi/dispatch.c b/src/uapi/dispatch.c
index 859543a..2eef5bc 100644
--- a/src/uapi/dispatch.c
+++ b/src/uapi/dispatch.c
@@ -71,7 +71,6 @@ void handle_syscall(sys_arg_t syscall, sys_arg_t a, sys_arg_t b,
 	case SYS_FORK: sys_fork(t, a, b, c, d, e); break;
 	case SYS_EXEC: sys_exec(t, a, b, c, d, e); break;
 	case SYS_SPAWN: sys_spawn(t, a, b, c, d, e); break;
-	case SYS_KILL: sys_kill(t, a, b, c, d, e); break;
 	case SYS_SWAP: sys_swap(t, a, b, c, d, e); break;
 	case SYS_SET_CONF: sys_set_conf(t, a, b, c, d, e); break;
 	case SYS_GET_CONF: sys_get_conf(t, a, b, c, d, e); break;
diff --git a/src/uapi/ipc.c b/src/uapi/ipc.c
index 850178c..0fba899 100644
--- a/src/uapi/ipc.c
+++ b/src/uapi/ipc.c
@@ -71,7 +71,7 @@ static inline void finalize_rpc(struct tcb *t, struct tcb *r, vm_t s)
 {
 	clone_uvmem(r->proc.vmem, t->rpc.vmem);
 	set_return(t, r->callback);
-	reference_proc(r);
+	reference_thread(r);
 	t->pid = r->rid;
 
 	/* make sure updates are visible when swapping to the new virtual memory */
@@ -446,6 +446,7 @@ SYSCALL_DEFINE4(ipc_resp)(struct tcb *t, sys_arg_t d0, sys_arg_t d1,
 	/* inform requester who answered (pid) in the case of the request being
 	 * kicked forward */
 	enable_irqs();
+	unreference_thread(get_cproc(t));
 	leave_rpc(t, SYS_RET6(OK, t->pid, d0, d1, d2, d3));
 }
 
diff --git a/src/uapi/proc.c b/src/uapi/proc.c
index c0fe5f0..b90433f 100644
--- a/src/uapi/proc.c
+++ b/src/uapi/proc.c
@@ -175,29 +175,6 @@ SYSCALL_DEFINE2(spawn)(struct tcb *t, sys_arg_t bin, sys_arg_t interp)
 }
 
 /**
- * Kill syscall handler.
- *
- * @param t Current tcb.
- * @param pid Process to kill.
- * \todo Implement.
- *
- * @return ERR_PERM if not capable to kill, otherwise OK.
- */
-SYSCALL_DEFINE1(kill)(struct tcb *t, sys_arg_t pid)
-{
-	struct tcb *c = get_cproc(t);
-	if (!(has_cap(c->caps, CAP_PROC)))
-		return_args1(t, ERR_PERM);
-
-	struct tcb *r = get_tcb(pid);
-	if (is_proc(r))
-		return_args1(t, ERR_INVAL);
-
-	destroy_proc(r);
-	return_args1(t, OK);
-}
-
-/**
  * Actual worker of swapping between threads.
  * Assumes that both \p t and \p s exist and that \p s isn't a zombie or
  * currently running.
@@ -215,7 +192,7 @@ static void swap(struct tcb *t, struct tcb *s)
 	enable_irqs();
 
 	if (!is_rpc(s) && orphan(s)) {
-		orphanize(s);
+		unorphanize(s);
 		return;
 	}
 
@@ -242,6 +219,12 @@ static void swap(struct tcb *t, struct tcb *s)
  */
 SYSCALL_DEFINE1(exit)(struct tcb *t, sys_arg_t tid)
 {
+	/* init thread is not allowed to exit */
+	/** @todo what about other possible threads start at init, are they
+	 * allowed to exit? I guess? */
+	if (t->tid == 1)
+		return_args1(t, ERR_INVAL);
+
 	if (tid != 0) {
 		struct tcb *s = get_tcb(tid);
 		if (!s)
@@ -284,17 +267,7 @@ SYSCALL_DEFINE1(detach)(struct tcb *t, sys_arg_t tid)
 	if (!o || orphan(o))
 		return_args1(t, ERR_INVAL);
 
-	struct tcb *r = get_tcb(o->rid);
-	if (r)
-		unreference_proc(r);
-
 	orphanize(o);
-
-	/* generally the thread shouldn't do anything with this information, but
-	 * it fits really nicely into the notification framework so just do it
-	 */
-	notify(o, NOTIFY_ORPHANED);
-
 	return_args1(t, OK);
 }
 
diff --git a/src/vmem.c b/src/vmem.c
index 6fb5474..96d4005 100644
--- a/src/vmem.c
+++ b/src/vmem.c
@@ -46,11 +46,11 @@ static stat_t __copy_mapped_region(struct tcb *d, struct tcb *s,
 	if (ERR_CODE(v))
 		return v;
 
+	assert(v == start);
+
 	if (is_set(m->flags, MR_NONBACKED))
 		return v;
 
-	assert(v == start);
-
 	/* note that we use uvmem.vmem instead of proc.vmem, this is just to
 	 * make sure that zombies don't eat our brains */
 	stat_t res = copy_region(d->uvmem.vmem, s->uvmem.vmem, v, v, size);
@@ -80,7 +80,7 @@ static stat_t __copy_shared_region(struct tcb *d, struct mem_region *m)
 	vm_t start = m->start * BASE_PAGE_SIZE;
 	vm_t end = m->end * BASE_PAGE_SIZE;
 
-	reference_proc(s);
+	reference_thread(s);
 
 	size_t size = end - start;
 	vm_t v = alloc_fixed_region(&d->uvmem.region, start, size, &size,
@@ -115,7 +115,7 @@ static vm_t __clone_shared_region(struct tcb *d, struct tcb *s,
 	vm_t start = m->start * BASE_PAGE_SIZE;
 	vm_t end = m->end * BASE_PAGE_SIZE;
 
-	reference_proc(s);
+	reference_thread(s);
 
 	size_t size = end - start;
 	vm_t v = alloc_shared_region(&d->uvmem.region, size, &size,
@@ -129,7 +129,7 @@ static vm_t __clone_shared_region(struct tcb *d, struct tcb *s,
 		return v;
 
 	/* cleanup on error */
-	unreference_proc(s);
+	unreference_thread(s);
 	free_region(&d->uvmem.region, v);
 	unmap_fixed_region(d->uvmem.vmem, v, size);
 	return res;
@@ -145,7 +145,7 @@ static void __free_mapping(struct tcb *t, struct mem_region *m)
 {
 	struct tcb *owner = get_tcb(m->pid);
 	if (owner)
-		unreference_proc(owner);
+		unreference_thread(owner);
 
 	if (is_set(m->flags, MR_NONBACKED))
 		return;