From dabd2355f355abd7d1d58641f6cc496adb1482d1 Mon Sep 17 00:00:00 2001
From: Alex Auvolat <alex@adnab.me>
Date: Sat, 14 Mar 2015 12:56:22 +0100
Subject: Add mutexes everywhere ; spam debug log (sorry)

---
 src/common/libalgo/hashtbl.c      |  3 +-
 src/common/libkogata/slab_alloc.c | 12 +++++++
 src/kernel/core/paging.c          |  5 +--
 src/kernel/core/sys.c             |  3 +-
 src/kernel/core/thread.c          | 26 +++++++--------
 src/kernel/include/vfs.h          |  3 ++
 src/kernel/user/ipc.c             |  7 +++++
 src/kernel/user/process.c         | 66 ++++++++++++++++++++++++---------------
 src/kernel/user/syscall.c         |  2 +-
 src/kernel/user/vfs.c             | 34 ++++++++++++++++++++
 10 files changed, 118 insertions(+), 43 deletions(-)

diff --git a/src/common/libalgo/hashtbl.c b/src/common/libalgo/hashtbl.c
index 13185c2..6fb5b60 100644
--- a/src/common/libalgo/hashtbl.c
+++ b/src/common/libalgo/hashtbl.c
@@ -1,3 +1,4 @@
+#include <debug.h>
 #include <malloc.h>
 
 #include <hashtbl.h>
@@ -68,7 +69,7 @@ void hashtbl_check_size(hashtbl_t *ht) {
 
 	if (nsize != 0) {
 		hashtbl_item_t **nitems = (hashtbl_item_t**)malloc(nsize * sizeof(hashtbl_item_t*));
-		if (nitems == 0) return;	// if we can't realloc, too bad, we just lose space
+		if (nitems == 0) return;	// if we can't realloc, too bad, we just lose space/efficienty
 
 		for (size_t i = 0; i < nsize; i++) nitems[i] = 0;
 
diff --git a/src/common/libkogata/slab_alloc.c b/src/common/libkogata/slab_alloc.c
index 837a965..daa3a50 100644
--- a/src/common/libkogata/slab_alloc.c
+++ b/src/common/libkogata/slab_alloc.c
@@ -235,6 +235,18 @@ void slab_free(mem_allocator_t* a, void* addr) {
 				ASSERT((addr - r->region_addr) % a->types[i].obj_size == 0);
 
 				object_t *o = (object_t*)addr;
+
+				// check the object is not already on the free list (double-free error)
+				for (object_t *i = r->first_free_obj; i != 0; i = i->next) {
+					if (!((void*)i >= r->region_addr && (void*)i < r->region_addr + region_size)){
+						dbg_printf("Invalid object 0x%p in cache 0x%p - %x\n", i, r->region_addr, region_size);
+						PANIC("Error");
+					}
+					ASSERT(o != i);
+				}
+
+				/*dbg_printf("Put back 0x%p in 0x%p\n", o, r->region_addr);*/
+
 				o->next = r->first_free_obj;
 				r->first_free_obj = o;
 				r->n_free_objs++;
diff --git a/src/kernel/core/paging.c b/src/kernel/core/paging.c
index f7ebf9c..6233443 100644
--- a/src/kernel/core/paging.c
+++ b/src/kernel/core/paging.c
@@ -58,9 +58,10 @@ void page_fault_handler(registers_t *regs) {
 			current_thread->user_ex_handler(regs);
 		} else {
 			if (pd->user_pfh == 0) {
-				dbg_printf("Error: usermode page fault on PD with no user PFH.\n");
+				dbg_printf("Error: usermode page fault (0x%p) on PD with no user PFH.\n", vaddr);
 				dbg_printf("PD: 0x%p, kernel PD: 0x%p\n", get_current_pagedir(), get_kernel_pagedir());
-				PANIC("Un-handlable usermode PF.\n");
+				dbg_dump_registers(regs);
+				PANIC("Un-handlable usermode PF.");
 			}
 			ASSERT(pd->user_pfh != 0);
 			pd->user_pfh(pd->user_pfh_data, regs, vaddr);
diff --git a/src/kernel/core/sys.c b/src/kernel/core/sys.c
index 290fdfa..333cb58 100644
--- a/src/kernel/core/sys.c
+++ b/src/kernel/core/sys.c
@@ -71,7 +71,7 @@ void load_kernel_symbol_map(char* text, size_t len) {
 
 void kernel_stacktrace(uint32_t ebp, uint32_t eip) {
 	int i = 0;
-	while (ebp >= K_HIGHHALF_ADDR && eip >= K_HIGHHALF_ADDR) {
+	while (ebp >= K_HIGHHALF_ADDR) {
 		char* sym = 0;
 		if (kernel_symbol_map != 0) sym = btree_lower(kernel_symbol_map, (void*)eip);
 
@@ -84,6 +84,7 @@ void kernel_stacktrace(uint32_t ebp, uint32_t eip) {
 			dbg_printf("| ...");
 			break;
 		}
+		if (eip == 0) break;
 	}
 }
 
diff --git a/src/kernel/core/thread.c b/src/kernel/core/thread.c
index 53b3e70..c4d4f93 100644
--- a/src/kernel/core/thread.c
+++ b/src/kernel/core/thread.c
@@ -120,7 +120,7 @@ void run_scheduler() {
 	// At this point, interrupts are disabled
 	// This function is expected NEVER TO RETURN
 
-	/*thread_t *prev_thread = current_thread;*/
+	thread_t *prev_thread = current_thread;
 
 	if (current_thread != 0 && current_thread->state == T_STATE_RUNNING) {
 		current_thread->last_ran = get_kernel_time();
@@ -129,7 +129,7 @@ void run_scheduler() {
 	}
 	current_thread = dequeue_thread();
 
-	/*if (current_thread != prev_thread) dbg_printf("[0x%p]\n", current_thread);*/
+	if (current_thread != prev_thread) dbg_printf("[0x%p]\n", current_thread);
 
 	if (current_thread != 0) {
 		thread_t *ptr = current_thread;
@@ -345,19 +345,19 @@ void usleep(int usecs) {
 	if (ok) wait_on(current_thread);
 }
 
-void exit() {
-	void exit_cleanup_task(void* v) {
-		thread_t *t = (thread_t*)v;
-
-		if (t->proc == 0) {
-			// stand alone thread, can be deleted safely
-			delete_thread(t);
-		} else {
-			// call specific routine from process code
-			process_thread_exited(t);
-		}
+void exit_cleanup_task(void* v) {
+	thread_t *t = (thread_t*)v;
+
+	if (t->proc == 0) {
+		// stand alone thread, can be deleted safely
+		delete_thread(t);
+	} else {
+		// call specific routine from process code
+		process_thread_exited(t);
 	}
+}
 
+void exit() {
 	int st = enter_critical(CL_NOSWITCH);
 	// the critical section here does not guarantee that worker_push will return immediately
 	// (it may switch before adding the delete_thread task), but once the task is added
diff --git a/src/kernel/include/vfs.h b/src/kernel/include/vfs.h
index 226f962..8f09d7f 100644
--- a/src/kernel/include/vfs.h
+++ b/src/kernel/include/vfs.h
@@ -55,6 +55,7 @@ typedef struct fs_handle {
 	struct fs_node *node;
 
 	int refs;
+	mutex_t lock;
 
 	int mode;
 
@@ -125,6 +126,8 @@ typedef struct {
 typedef struct fs {
 	// Filled by VFS's make_fs()
 	int refs;
+	mutex_t lock;
+
 	struct fs *from_fs;
 	int ok_modes;
 	// Filled by FS's specific make() - all zero in the case of a subfs
diff --git a/src/kernel/user/ipc.c b/src/kernel/user/ipc.c
index 7f34fdf..308a9c1 100644
--- a/src/kernel/user/ipc.c
+++ b/src/kernel/user/ipc.c
@@ -83,6 +83,10 @@ fs_handle_pair_t make_channel(bool blocking) {
 	ret.b->node = nb;
 	ret.a->refs = ret.b->refs = 1;
 	ret.a->mode = ret.b->mode = FM_READ | FM_WRITE | (blocking ? FM_BLOCKING : 0);
+	ret.a->lock = ret.b->lock = MUTEX_UNLOCKED;
+
+	dbg_printf("hREF1c0x%p\n", ret.a);
+	dbg_printf("hREF1c0x%p\n", ret.b);
 
 	return ret;
 
@@ -274,8 +278,11 @@ fs_handle_t* make_shm(size_t size) {
 	h->fs = 0;
 	h->node = n;
 	h->refs = 1;
+	h->lock = MUTEX_UNLOCKED;
 	h->mode = FM_READ | FM_WRITE | FM_MMAP;
 
+	dbg_printf("hREF1s0x%p\n", h);
+
 	return h;
 
 error:
diff --git a/src/kernel/user/process.c b/src/kernel/user/process.c
index 2a4d98f..f56998e 100644
--- a/src/kernel/user/process.c
+++ b/src/kernel/user/process.c
@@ -9,6 +9,8 @@
 
 static int next_pid = 1;
 
+void munmap_inner(process_t *proc, user_region_t *r);
+
 void proc_user_exception(registers_t *regs);
 void proc_usermem_pf(void* proc, registers_t *regs, void* addr);
 
@@ -154,20 +156,21 @@ bool start_process(process_t *p, void* entry) {
 	return true;
 }
 
-void current_process_exit(int status, int exit_code) {
-	void process_exit_v(void* args) {
-		exit_data_t *d = (exit_data_t*)args;
-		process_exit(d->proc, d->status, d->exit_code);
-		free(d);
-	}
+void current_process_exit_task(void* args) {
+	exit_data_t *d = (exit_data_t*)args;
+	process_exit(d->proc, d->status, d->exit_code);
+	free(d);
+}
 
-	exit_data_t *d = (exit_data_t*)malloc(sizeof(exit_data_t));
+void current_process_exit(int status, int exit_code) {
+	exit_data_t *d;
+	while ((d = (exit_data_t*)malloc(sizeof(exit_data_t))) == 0) yield();
 
 	d->proc = current_process();;
 	d->status = status;
 	d->exit_code = exit_code;
 
-	while (!worker_push(process_exit_v, d)) yield();
+	while (!worker_push(current_process_exit_task, d)) yield();
 
 	exit();
 }
@@ -207,8 +210,9 @@ void process_exit(process_t *p, int status, int exit_code) {
 		process_t *ch = p->children;
 		p->children = ch->next_child;
 
-		if (ch->status == PS_RUNNING || ch->status == PS_LOADING)
+		if (ch->status == PS_RUNNING || ch->status == PS_LOADING) {
 			process_exit(ch, PS_KILLED, 0);
+		}
 		free(ch);
 	}
 
@@ -222,7 +226,7 @@ void process_exit(process_t *p, int status, int exit_code) {
 
 	// unmap memory
 	while (p->regions != 0) {
-		munmap(p, p->regions->addr);
+		munmap_inner(p, p->regions);
 	}
 	ASSERT(btree_count(p->regions_idx) == 0);
 	delete_btree(p->regions_idx);
@@ -441,6 +445,7 @@ bool proc_add_fs(process_t *p, fs_t *fs, const char* name) {
 
 	mutex_lock(&p->lock);
 
+	if (p->filesystems == 0) goto end;
 	if (hashtbl_find(p->filesystems, n) != 0) goto end;
 
 	add_ok = hashtbl_add(p->filesystems, n, fs);
@@ -458,7 +463,7 @@ end:
 fs_t *proc_find_fs(process_t *p, const char* name) {
 	mutex_lock(&p->lock);
 
-	fs_t *ret = (fs_t*)hashtbl_find(p->filesystems, name);
+	fs_t *ret = (p->filesystems != 0 ? (fs_t*)hashtbl_find(p->filesystems, name) : 0);
 
 	mutex_unlock(&p->lock);
 	return ret;
@@ -477,11 +482,10 @@ void proc_rm_fs(process_t *p, const char* name) {
 }
 
 int proc_add_fd(process_t *p, fs_handle_t *f) {
-	int fd = p->next_fd++;
-
 	mutex_lock(&p->lock);
 
-	bool add_ok = hashtbl_add(p->files, (void*)fd, f);
+	int fd = p->next_fd++;
+	bool add_ok = (p->files != 0 ? hashtbl_add(p->files, (void*)fd, f) : false);
 
 	mutex_unlock(&p->lock);
 
@@ -497,6 +501,7 @@ bool proc_add_fd_as(process_t *p, fs_handle_t *f, int fd) {
 
 	mutex_lock(&p->lock);
 
+	if (p->files == 0) goto end;
 	if (hashtbl_find(p->files, (void*)fd) != 0) goto end;
 
 	if (fd >= p->next_fd) p->next_fd = fd + 1;
@@ -511,7 +516,7 @@ end:
 fs_handle_t *proc_read_fd(process_t *p, int fd) {
 	mutex_lock(&p->lock);
 
-	fs_handle_t *ret = (fs_handle_t*)hashtbl_find(p->files, (void*)fd);
+	fs_handle_t *ret = (p->files != 0 ? (fs_handle_t*)hashtbl_find(p->files, (void*)fd) : 0);
 
 	mutex_unlock(&p->lock);
 	return ret;
@@ -520,7 +525,7 @@ fs_handle_t *proc_read_fd(process_t *p, int fd) {
 void proc_close_fd(process_t *p, int fd) {
 	mutex_lock(&p->lock);
 
-	fs_handle_t *x = (fs_handle_t*)hashtbl_find(p->files, (void*)fd);
+	fs_handle_t *x = (p->files != 0 ? (fs_handle_t*)hashtbl_find(p->files, (void*)fd) : 0);
 	if (x != 0) {
 		unref_file(x);
 		hashtbl_remove(p->files, (void*)fd);
@@ -536,7 +541,7 @@ void proc_close_fd(process_t *p, int fd) {
 user_region_t *find_user_region(process_t *proc, const void* addr) {
 	mutex_lock(&proc->lock);
 
-	user_region_t *r = (user_region_t*)btree_lower(proc->regions_idx, addr);
+	user_region_t *r = (proc->regions_idx != 0 ? (user_region_t*)btree_lower(proc->regions_idx, addr) : 0);
 
 	if (r != 0) {
 		ASSERT(addr >= r->addr);
@@ -561,6 +566,8 @@ bool mmap(process_t *proc, void* addr, size_t size, int mode) {
 
 	mutex_lock(&proc->lock);
 
+	if (proc->regions_idx == 0) goto error;
+
 	r->proc = proc;
 	r->addr = addr;
 	r->size = size;
@@ -592,6 +599,7 @@ error:
 }
 
 bool mmap_file(process_t *proc, fs_handle_t *h, size_t offset, void* addr, size_t size, int mode) {
+	dbg_printf("Mmap file 0x%p\n", h);
 	if (find_user_region(proc, addr) != 0) return false;
 
 	if ((uint32_t)addr & (~PAGE_MASK)) return false;
@@ -611,6 +619,8 @@ bool mmap_file(process_t *proc, fs_handle_t *h, size_t offset, void* addr, size_
 
 	mutex_lock(&proc->lock);
 
+	if (proc->regions_idx == 0) goto error;
+
 	r->addr = addr;
 	r->size = size;
 
@@ -673,12 +683,9 @@ bool mchmap(process_t *proc, void* addr, int mode) {
 	return true;
 }
 
-bool munmap(process_t *proc, void* addr) {
-	user_region_t *r = find_user_region(proc, addr);
-	if (r == 0) return false;
-
-	mutex_lock(&proc->lock);
-
+void munmap_inner(process_t *proc, user_region_t *r) {
+	// assume theprocess lock is locked
+	// will always succeed
 	if (proc->regions == r) {
 		proc->regions = r->next_in_proc;
 	} else {
@@ -707,14 +714,23 @@ bool munmap(process_t *proc, void* addr) {
 	}
 	switch_pagedir(save_pd);
 
-	mutex_unlock(&proc->lock);
-
 	pager_rm_map(r->pager, r);
 
 	if (r->file != 0) unref_file(r->file);
 	if (r->own_pager) delete_pager(r->pager);
 
 	free(r);
+}
+
+bool munmap(process_t *proc, void* addr) {
+	user_region_t *r = find_user_region(proc, addr);
+	if (r == 0) return false;
+
+	mutex_lock(&proc->lock);
+
+	munmap_inner(proc, r);
+
+	mutex_unlock(&proc->lock);
 
 	return true;
 }
diff --git a/src/kernel/user/syscall.c b/src/kernel/user/syscall.c
index 350262c..42b06eb 100644
--- a/src/kernel/user/syscall.c
+++ b/src/kernel/user/syscall.c
@@ -307,7 +307,7 @@ uint32_t select_sc(sc_args_t args) {
 	uint64_t select_begin_time = get_kernel_time();
 
 	void** wait_objs = (void**)malloc((n+1) * sizeof(void*));
-	if (!wait_objs) return false;
+	if (wait_objs == 0) return false;
 
 	bool ret = false;
 
diff --git a/src/kernel/user/vfs.c b/src/kernel/user/vfs.c
index a967f70..db01888 100644
--- a/src/kernel/user/vfs.c
+++ b/src/kernel/user/vfs.c
@@ -42,6 +42,7 @@ fs_t *make_fs(const char* drv_name, fs_handle_t *source, const char* opts) {
 	if (fs->root == 0) goto fail;
 
 	fs->refs = 1;
+	fs->lock = MUTEX_UNLOCKED;
 	fs->from_fs = 0;
 	fs->ok_modes = FM_ALL_MODES;
 	fs->root->refs = 1;
@@ -49,6 +50,8 @@ fs_t *make_fs(const char* drv_name, fs_handle_t *source, const char* opts) {
 	fs->root->parent = 0;
 	fs->root->children = 0;
 
+	dbg_printf("sREF1m0x%p\n", fs);
+
 	// Look for driver
 	for(fs_driver_t *i = drivers; i != 0; i = i->next) {
 		if ((drv_name != 0 && strcmp(i->name, drv_name) == 0) || (drv_name == 0 && source != 0)) {
@@ -75,6 +78,8 @@ fs_t *fs_subfs(fs_t *fs, const char* root, int ok_modes) {
 	if (subfs == 0) return 0;
 
 	subfs->refs = 1;
+	subfs->lock = MUTEX_UNLOCKED;
+
 	subfs->from_fs = fs;
 	subfs->ok_modes = ok_modes & fs->ok_modes;
 
@@ -83,6 +88,8 @@ fs_t *fs_subfs(fs_t *fs, const char* root, int ok_modes) {
 
 	subfs->root = new_root;
 
+	dbg_printf("sREF1s0x%p\n", fs);
+
 	return subfs;
 }
 
@@ -91,10 +98,20 @@ bool fs_add_source(fs_t *fs, fs_handle_t *source, const char* opts) {
 }
 
 void ref_fs(fs_t *fs) {
+	dbg_printf("sREF++0x%p(%d)\n", fs, fs->refs);
+
+	mutex_lock(&fs->lock);
+	
 	fs->refs++;
+
+	mutex_unlock(&fs->lock);
 }
 
 void unref_fs(fs_t *fs) {
+	dbg_printf("sREF--0x%p(%d)\n", fs, fs->refs);
+
+	mutex_lock(&fs->lock);
+
 	fs->refs--;
 	if (fs->refs == 0) {
 		if (fs->from_fs != 0) {
@@ -105,6 +122,8 @@ void unref_fs(fs_t *fs) {
 		}
 		if (fs->ops->shutdown) fs->ops->shutdown(fs->data);
 		free(fs);
+	} else {
+		mutex_unlock(&fs->lock);
 	}
 }
 
@@ -432,10 +451,13 @@ fs_handle_t* fs_open(fs_t *fs, const char* file, int mode) {
 	if (!open_ok) goto error;
 
 	h->refs = 1;
+	h->lock = MUTEX_UNLOCKED;
 	h->fs = fs;
 	h->node = n;
 	h->mode = mode;
 
+	dbg_printf("hREF1o0x%p\n", h);
+
 	// our reference to node n is transferred to the file handle
 	mutex_unlock(&n->lock);
 	ref_fs(fs);
@@ -449,16 +471,28 @@ error:
 }
 
 void ref_file(fs_handle_t *file) {
+	dbg_printf("hREF++0x%p(%d)\n", file, file->refs);
+
+	mutex_lock(&file->lock);
+
 	file->refs++;
+
+	mutex_unlock(&file->lock);
 }
 
 void unref_file(fs_handle_t *file) {
+	dbg_printf("hREF--0x%p(%d)\n", file, file->refs);
+
+	mutex_lock(&file->lock);
+
 	file->refs--;
 	if (file->refs == 0) {
 		if (file->node->ops->close) file->node->ops->close(file);
 		unref_fs_node(file->node);
 		if (file->fs) unref_fs(file->fs);
 		free(file);
+	} else {
+		mutex_unlock(&file->lock);
 	}
 }
 
-- 
cgit v1.2.3