fix: remove malloc in mutex and update tests

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

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;
 
@@ -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;
+    STAILQ_INIT(&mutex->fifo);
-    if (mutex_fifo_hashmap[id] == NULL)
-    {
-        mutex_fifo_hashmap[id] = malloc(sizeof(mutex_fifo));
-        STAILQ_INIT(mutex_fifo_hashmap[id]);
-    }
 
     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 %p\n", mutex);
-    DBG("Lock mutex %d\n", id);
     while (! __sync_bool_compare_and_swap(&mutex->dummy, 0, 1))
     {
-        DBG("Wait for mutex %d\n", id);
+        DBG("Wait for mutex %p\n", mutex);
-        if (running->mutex_fifo_entry == NULL)
+        STAILQ_INSERT_TAIL(&mutex->fifo, &running->mutex_fifo_entry, link);
-            running->mutex_fifo_entry = malloc(sizeof(struct mutex_fifo_entry_t));
 
-        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 %p\n", mutex);
-    DBG("Unlock mutex %d\n", id);
+    if (!STAILQ_EMPTY(&mutex->fifo))
-    if (!STAILQ_EMPTY(mutex_fifo_hashmap[id]))
     {
-        struct mutex_fifo_entry_t* first = STAILQ_FIRST(mutex_fifo_hashmap[id]);
+        struct mutex_fifo_entry_t* first = STAILQ_FIRST(&mutex->fifo);
-        STAILQ_REMOVE_HEAD(mutex_fifo_hashmap[id], link);
+        STAILQ_REMOVE_HEAD(&mutex->fifo, link);
 
         UNSET_STATUS(first->thread, MUTEX_WAITING);
         TAILQ_INSERT_TAIL(&head, first->thread, link);

src/thread/thread.h

@@ -2,6 +2,7 @@
 #define __THREAD_H__
 
 #ifndef USE_PTHREAD
+#include <sys/queue.h>
 
 /* 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);

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("terminé OK\n");
-  printf("%lu us\n", us);
   return EXIT_SUCCESS;
 }

tst/64-mutex-join.c

@@ -6,14 +6,15 @@
 #include <sys/time.h>
 #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("terminé OK\n");
-  printf("%lu us\n", us);
   return EXIT_SUCCESS;
 }