From e45992ac9da1003ff0acb7d1f260b672e2f4a4aa Mon Sep 17 00:00:00 2001 From: Nemo D'ACREMONT Date: Fri, 4 Apr 2025 16:52:14 +0200 Subject: [PATCH] fix: remove malloc in mutex and update tests --- Makefile | 4 +-- src/thread/thread.c | 59 ++++++++----------------------------------- src/thread/thread.h | 8 +++++- tst/63-mutex-equity.c | 21 +++++++-------- tst/64-mutex-join.c | 13 +++------- 5 files changed, 35 insertions(+), 70 deletions(-) diff --git a/Makefile b/Makefile index bd38880..59e1fda 100644 --- a/Makefile +++ b/Makefile @@ -29,7 +29,7 @@ install_bins_targets+=${install_dir}/bin/51-fibonacci valgrind_targets=$(addprefix valgrind_,${bins}) -check_argv?=8 255 +check_argv?=20 255 check_targets=$(addprefix check_,${bins}) src_dirs=$(sort $(dir $(wildcard ${src_dir}/**/))) @@ -88,7 +88,7 @@ valgrind: ${valgrind_targets} .PHONY: ${valgrind_targets} ${valgrind_targets}: valgrind_%: ${build_dir}/% - valgrind $^ --leak-check=full --show-reachable=yes --track-origins=yes + valgrind $^ ${check_argv} --leak-check=full --show-reachable=yes --track-origins=yes .PHONY: build build: ${bins_target} ${build_dir}/libthread.so ${build_dir}/libthread.a diff --git a/src/thread/thread.c b/src/thread/thread.c index 5b71e06..ba788e3 100644 --- a/src/thread/thread.c +++ b/src/thread/thread.c @@ -35,11 +35,6 @@ #define STACK_SIZE 4096 #endif -// Variables used to clean up everything at the end of the processes -#ifndef HASHMAP_SIZE -#define HASHMAP_SIZE 16384 -#endif - // Variables used to clean up everything at the end of the processus static char stack_for_freeing[STACK_SIZE] = {0}; static int stack_valgrind_id = 0; @@ -54,7 +49,7 @@ struct context_entry { ucontext_t context; void *retvalue; // return value or if the thread is waited, the id of the thread that wait for it struct last_thread_t *last_waited; - struct mutex_fifo_entry_t* mutex_fifo_entry; + struct mutex_fifo_entry_t mutex_fifo_entry; int valgrind_id; char status; char stack[STACK_SIZE]; @@ -76,12 +71,6 @@ static TAILQ_HEAD(freed_context_head, context_entry) context_to_freed = TAILQ_HE static STAILQ_HEAD(last_thread_head, last_thread_t) last_thread_freed = STAILQ_HEAD_INITIALIZER(last_thread_freed); -struct mutex_fifo_entry_t { - STAILQ_ENTRY(mutex_fifo_entry_t) link; - struct context_entry* thread; -}; -STAILQ_HEAD(mutex_fifo, mutex_fifo_entry_t) mutex_fifo; -static struct mutex_fifo* mutex_fifo_hashmap[HASHMAP_SIZE] = {}; int thread_yield(void) { @@ -165,7 +154,6 @@ int thread_create(thread_t* newthread, void* (*func)(void*), void* funcarg) new_entry->status = ALLOCATED; new_entry->retvalue = NULL; new_entry->last_waited = NULL; - new_entry->mutex_fifo_entry = NULL; *newthread = new_entry; @@ -239,7 +227,7 @@ int thread_join(thread_t thread, void** retval) } } - + DBG("%p is waiting for %p", running, entry); DBG("MUTEX WAITING %d", IS_MUTEX_WAITING(GET_LAST_WAITED_THREAD(running))); @@ -315,7 +303,6 @@ void clear_context(void) while (!TAILQ_EMPTY(&head)) { last = TAILQ_FIRST(&head); TAILQ_REMOVE(&head, last, link); - free(last->mutex_fifo_entry); if (WAS_ALLOCATED(last)) { VALGRIND_STACK_DEREGISTER(last->valgrind_id); } @@ -327,7 +314,6 @@ void clear_context(void) } while (!TAILQ_EMPTY(&context_to_freed)) { last = TAILQ_FIRST(&context_to_freed); - free(last->mutex_fifo_entry); TAILQ_REMOVE(&context_to_freed, last, link); if (WAS_ALLOCATED(last)) { VALGRIND_STACK_DEREGISTER(last->valgrind_id); @@ -342,10 +328,6 @@ void clear_context(void) free(last_thread); } - // Free all the fifo that might have been allocated - for (int i = 0 ; i < HASHMAP_SIZE ; ++i) - free(mutex_fifo_hashmap[i]); - VALGRIND_STACK_DEREGISTER(stack_valgrind_id); exit(0); } @@ -361,7 +343,6 @@ void __attribute__((constructor)) setup_main_thread() main->valgrind_id = 0; main->retvalue = NULL; main->last_waited = NULL; - main->mutex_fifo_entry = NULL; running = main; // Create a context with static stack to clean everything at the end. @@ -390,45 +371,28 @@ void __attribute__((destructor)) clear_last_thread() int thread_mutex_init(thread_mutex_t* mutex) { - long id = ((long)mutex) % HASHMAP_SIZE; - if (mutex_fifo_hashmap[id] == NULL) - { - mutex_fifo_hashmap[id] = malloc(sizeof(mutex_fifo)); - STAILQ_INIT(mutex_fifo_hashmap[id]); - } + STAILQ_INIT(&mutex->fifo); return mutex->dummy = 0; } int thread_mutex_destroy(thread_mutex_t* mutex) { - long id = ((long)mutex) % HASHMAP_SIZE; - struct mutex_fifo_entry_t* last = NULL; - - while (!STAILQ_EMPTY(mutex_fifo_hashmap[id])) { - last = STAILQ_FIRST(mutex_fifo_hashmap[id]); - STAILQ_REMOVE_HEAD(mutex_fifo_hashmap[id], link); - free(last); - } - return 0; } int thread_mutex_lock(thread_mutex_t* mutex) { // Add to mutex fifo - long id = ((long)mutex) % HASHMAP_SIZE; - DBG("Lock mutex %d\n", id); + DBG("Lock mutex %p\n", mutex); while (! __sync_bool_compare_and_swap(&mutex->dummy, 0, 1)) { - DBG("Wait for mutex %d\n", id); - if (running->mutex_fifo_entry == NULL) - running->mutex_fifo_entry = malloc(sizeof(struct mutex_fifo_entry_t)); + DBG("Wait for mutex %p\n", mutex); + STAILQ_INSERT_TAIL(&mutex->fifo, &running->mutex_fifo_entry, link); - STAILQ_INSERT_TAIL(mutex_fifo_hashmap[id], running->mutex_fifo_entry, link); // Use status to be in waiting state SET_STATUS(running, MUTEX_WAITING); - running->mutex_fifo_entry->thread = running; + running->mutex_fifo_entry.thread = running; thread_yield(); } @@ -438,12 +402,11 @@ int thread_mutex_lock(thread_mutex_t* mutex) int thread_mutex_unlock(thread_mutex_t* mutex) { - long id = ((long)mutex) % HASHMAP_SIZE; - DBG("Unlock mutex %d\n", id); - if (!STAILQ_EMPTY(mutex_fifo_hashmap[id])) + DBG("Unlock mutex %p\n", mutex); + if (!STAILQ_EMPTY(&mutex->fifo)) { - struct mutex_fifo_entry_t* first = STAILQ_FIRST(mutex_fifo_hashmap[id]); - STAILQ_REMOVE_HEAD(mutex_fifo_hashmap[id], link); + struct mutex_fifo_entry_t* first = STAILQ_FIRST(&mutex->fifo); + STAILQ_REMOVE_HEAD(&mutex->fifo, link); UNSET_STATUS(first->thread, MUTEX_WAITING); TAILQ_INSERT_TAIL(&head, first->thread, link); diff --git a/src/thread/thread.h b/src/thread/thread.h index 820c7d0..3807e50 100644 --- a/src/thread/thread.h +++ b/src/thread/thread.h @@ -2,6 +2,7 @@ #define __THREAD_H__ #ifndef USE_PTHREAD +#include /* identifiant de thread * NB: pourra être un entier au lieu d'un pointeur si ca vous arrange, @@ -40,7 +41,12 @@ extern int thread_join(thread_t thread, void **retval); extern void thread_exit(void *retval) __attribute__ ((__noreturn__)); /* Interface possible pour les mutex */ -typedef struct thread_mutex { int dummy; } thread_mutex_t; +struct mutex_fifo_entry_t { + STAILQ_ENTRY(mutex_fifo_entry_t) link; + struct context_entry* thread; +}; +STAILQ_HEAD(mutex_fifo_t, mutex_fifo_entry_t); +typedef struct thread_mutex { int dummy; struct mutex_fifo_t fifo; } thread_mutex_t; int thread_mutex_init(thread_mutex_t *mutex); int thread_mutex_destroy(thread_mutex_t *mutex); int thread_mutex_lock(thread_mutex_t *mutex); diff --git a/tst/63-mutex-equity.c b/tst/63-mutex-equity.c index c450ab8..20c1641 100644 --- a/tst/63-mutex-equity.c +++ b/tst/63-mutex-equity.c @@ -7,13 +7,14 @@ #include "thread.h" /* test pour verifier que prendre un mutex ne desactive pas l'equité envers les autres threads. + * si ca deadlock, c'est qu'il y a un problème. + * la durée du test n'est pas clairement utilisable pour juger de l'équité. * * valgrind doit etre content. * Le programme doit finir. * * support nécessaire: * - thread_create() - * - retour sans thread_exit() * - thread_join() sans récupération de la valeur de retour * - thread_mutex_init() * - thread_mutex_destroy() @@ -30,14 +31,17 @@ static void * thfunc(void *dummy __attribute__((unused))) /* on incremente progressivement fini jusque 5 pour debloquer le main */ for(i=0; i<5; i++) { + printf(" le fils yield sans le mutex et incrémente le compteur %u\n", fini); thread_yield(); fini++; } /* on attend que main remette à 0 */ thread_mutex_lock(&lock); - while (fini != 0) + while (fini != 0) { + printf(" le fils yield avec le mutex en attendant que le compteur %u soit 0\n", fini); thread_yield(); + } thread_mutex_unlock(&lock); thread_exit(NULL); @@ -47,10 +51,6 @@ int main(void) { thread_t th; int err, i; - struct timeval tv1, tv2; - unsigned long us; - - gettimeofday(&tv1, NULL); /* on cree le mutex et le thread */ if (thread_mutex_init(&lock) != 0) { @@ -62,12 +62,15 @@ int main(void) /* on prend le lock puis on attend que l'autre mette fini = 5 */ thread_mutex_lock(&lock); - while (fini != 5) + while (fini != 5) { + printf("le père yield avec le mutex en attendant que le compteur %u soit 5\n", fini); thread_yield(); + } thread_mutex_unlock(&lock); /* on baisse progressivement jusque 0 */ for(i=0; i<5; i++) { + printf("le père yield sans le mutex et décrémente le compteur %u\n", fini); thread_yield(); fini--; } @@ -75,10 +78,8 @@ int main(void) /* fini */ err = thread_join(th, NULL); assert(!err); - gettimeofday(&tv2, NULL); thread_mutex_destroy(&lock); - us = (tv2.tv_sec-tv1.tv_sec)*1000000+(tv2.tv_usec-tv1.tv_usec); - printf("%lu us\n", us); + printf("terminé OK\n"); return EXIT_SUCCESS; } diff --git a/tst/64-mutex-join.c b/tst/64-mutex-join.c index 9f10e71..1e738bd 100644 --- a/tst/64-mutex-join.c +++ b/tst/64-mutex-join.c @@ -6,14 +6,15 @@ #include #include "thread.h" -/* test pour verifier que prendre un mutex ne casse pas le join +/* test pour verifier que prendre un mutex ne casse pas le join. + * si ca deadlock, c'est qu'il y a un problème. + * la durée du test n'est pas clairement utilisable pour juger de la qualité de l'implementation. * * valgrind doit etre content. * Le programme doit finir. * * support nécessaire: * - thread_create() - * - retour sans thread_exit() * - thread_join() dans un mutex * - thread_mutex_init() * - thread_mutex_destroy() @@ -55,12 +56,8 @@ int main(void) { thread_t th; int err; - struct timeval tv1, tv2; - unsigned long us; void *ret; - gettimeofday(&tv1, NULL); - /* on cree le mutex et le thread */ if (thread_mutex_init(&lock) != 0) { fprintf(stderr, "thread_mutex_init failed\n"); @@ -83,12 +80,10 @@ int main(void) assert(pret == 2); printf("pere: join réussi\n"); - gettimeofday(&tv2, NULL); err = thread_mutex_unlock(&lock); assert(!err); thread_mutex_destroy(&lock); - us = (tv2.tv_sec-tv1.tv_sec)*1000000+(tv2.tv_usec-tv1.tv_usec); - printf("%lu us\n", us); + printf("terminé OK\n"); return EXIT_SUCCESS; }