Revert "revert r65471 and include Eric's patch as well"

This reverts commit ff5dc2cbbf9e7b67c8579ef166bf6a4755507304.

Deadlock: http://ci.rvm.jp/results/trunk-mjit@silicon-docker/1438883

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@65474 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

2 files changed, 130 insertions, 73 deletions

diff --git a/mjit.c b/mjit.c
index 9bbc94c1fb..3cb8f1daf1 100644
--- a/mjit.c
+++ b/mjit.c
@@ -119,7 +119,7 @@ mjit_free_iseq(const rb_iseq_t *iseq)
 static void
 init_list(struct rb_mjit_unit_list *list)
 {
-    list_head_init(&list->head);
+    list->head = NULL;
     list->length = 0;
 }
 
@@ -129,11 +129,11 @@ init_list(struct rb_mjit_unit_list *list)
 static void
 free_list(struct rb_mjit_unit_list *list)
 {
-    struct rb_mjit_unit *unit = 0, *next;
-
-    list_for_each_safe(&list->head, unit, next, unode) {
-        list_del(&unit->unode);
-        free_unit(unit);
+    struct rb_mjit_unit_node *node, *next;
+    for (node = list->head; node != NULL; node = next) {
+        next = node->next;
+        free_unit(node->unit);
+        xfree(node);
     }
     list->length = 0;
 }
@@ -248,23 +248,24 @@ unload_units(void)
 {
     rb_vm_t *vm = GET_THREAD()->vm;
     rb_thread_t *th = NULL;
-    struct rb_mjit_unit *unit = 0, *next, *worst;
+    struct rb_mjit_unit_node *node, *next, *worst_node;
     struct mjit_cont *cont;
     int delete_num, units_num = active_units.length;
 
     /* For now, we don't unload units when ISeq is GCed. We should
        unload such ISeqs first here. */
-    list_for_each_safe(&active_units.head, unit, next, unode) {
-        if (unit->iseq == NULL) { /* ISeq is GCed. */
-            remove_from_list(unit, &active_units);
-            free_unit(unit);
+    for (node = active_units.head; node != NULL; node = next) {
+        next = node->next;
+        if (node->unit->iseq == NULL) { /* ISeq is GCed. */
+            free_unit(node->unit);
+            remove_from_list(node, &active_units);
         }
     }
 
     /* Detect units which are in use and can't be unloaded. */
-    list_for_each_safe(&active_units.head, unit, next, unode) {
-        assert(unit->iseq != NULL && unit->handle != NULL);
-        unit->used_code_p = FALSE;
+    for (node = active_units.head; node != NULL; node = node->next) {
+        assert(node->unit != NULL && node->unit->iseq != NULL && node->unit->handle != NULL);
+        node->unit->used_code_p = FALSE;
     }
     list_for_each(&vm->living_threads, th, vmlt_node) {
         mark_ec_units(th->ec);
@@ -279,23 +280,23 @@ unload_units(void)
     delete_num = active_units.length / 10;
     for (; active_units.length > mjit_opts.max_cache_size - delete_num;) {
         /* Find one unit that has the minimum total_calls. */
-        worst = NULL;
-        list_for_each_safe(&active_units.head, unit, next, unode) {
-            if (unit->used_code_p) /* We can't unload code on stack. */
+        worst_node = NULL;
+        for (node = active_units.head; node != NULL; node = node->next) {
+            if (node->unit->used_code_p) /* We can't unload code on stack. */
                 continue;
 
-            if (worst == NULL || worst->iseq->body->total_calls > unit->iseq->body->total_calls) {
-                worst = unit;
+            if (worst_node == NULL || worst_node->unit->iseq->body->total_calls > node->unit->iseq->body->total_calls) {
+                worst_node = node;
             }
         }
-        if (worst == NULL)
+        if (worst_node == NULL)
             break;
 
         /* Unload the worst node. */
-        verbose(2, "Unloading unit %d (calls=%lu)", worst->id, worst->iseq->body->total_calls);
-        assert(worst->handle != NULL);
-        remove_from_list(worst, &active_units);
-        free_unit(worst);
+        verbose(2, "Unloading unit %d (calls=%lu)", worst_node->unit->id, worst_node->unit->iseq->body->total_calls);
+        assert(worst_node->unit->handle != NULL);
+        free_unit(worst_node->unit);
+        remove_from_list(worst_node, &active_units);
     }
     verbose(1, "Too many JIT code -- %d units unloaded", units_num - active_units.length);
 }
@@ -305,6 +306,8 @@ unload_units(void)
 void
 mjit_add_iseq_to_process(const rb_iseq_t *iseq)
 {
+    struct rb_mjit_unit_node *node;
+
     if (!mjit_enabled || pch_status == PCH_FAILED)
         return;
 
@@ -314,8 +317,14 @@ mjit_add_iseq_to_process(const rb_iseq_t *iseq)
         /* Failure in creating the unit.  */
         return;
 
+    node = create_list_node(iseq->body->jit_unit);
+    if (node == NULL) {
+        mjit_warning("failed to allocate a node to be added to unit_queue");
+        return;
+    }
+
     CRITICAL_SECTION_START(3, "in add_iseq_to_process");
-    add_to_list(iseq->body->jit_unit, &unit_queue);
+    add_to_list(node, &unit_queue);
     if (active_units.length >= mjit_opts.max_cache_size) {
         unload_units();
     }
@@ -753,14 +762,14 @@ mjit_finish(void)
 void
 mjit_mark(void)
 {
-    struct rb_mjit_unit *unit = 0;
+    struct rb_mjit_unit_node *node;
     if (!mjit_enabled)
         return;
     RUBY_MARK_ENTER("mjit");
     CRITICAL_SECTION_START(4, "mjit_mark");
-    list_for_each(&unit_queue.head, unit, unode) {
-        if (unit->iseq) { /* ISeq is still not GCed */
-            VALUE iseq = (VALUE)unit->iseq;
+    for (node = unit_queue.head; node != NULL; node = node->next) {
+        if (node->unit->iseq) { /* ISeq is still not GCed */
+            VALUE iseq = (VALUE)node->unit->iseq;
             CRITICAL_SECTION_FINISH(4, "mjit_mark rb_gc_mark");
 
             /* Don't wrap critical section with this. This may trigger GC,
diff --git a/mjit_worker.c b/mjit_worker.c
index b8ebb29812..18ac81d6fc 100644
--- a/mjit_worker.c
+++ b/mjit_worker.c
@@ -139,12 +139,18 @@ struct rb_mjit_unit {
 #endif
     /* Only used by unload_units. Flag to check this unit is currently on stack or not. */
     char used_code_p;
-    struct list_node unode;
+};
+
+/* Node of linked list in struct rb_mjit_unit_list.
+   TODO: use ccan/list for this */
+struct rb_mjit_unit_node {
+    struct rb_mjit_unit *unit;
+    struct rb_mjit_unit_node *next, *prev;
 };
 
 /* Linked list of struct rb_mjit_unit.  */
 struct rb_mjit_unit_list {
-    struct list_head head;
+    struct rb_mjit_unit_node *head;
     int length; /* the list length */
 };
 
@@ -175,11 +181,11 @@ int mjit_call_p = FALSE;
 
 /* Priority queue of iseqs waiting for JIT compilation.
    This variable is a pointer to head unit of the queue. */
-static struct rb_mjit_unit_list unit_queue = { LIST_HEAD_INIT(unit_queue.head) };
+static struct rb_mjit_unit_list unit_queue;
 /* List of units which are successfully compiled. */
-static struct rb_mjit_unit_list active_units = { LIST_HEAD_INIT(active_units.head) };
+static struct rb_mjit_unit_list active_units;
 /* List of compacted so files which will be deleted in `mjit_finish()`. */
-static struct rb_mjit_unit_list compact_units = { LIST_HEAD_INIT(compact_units.head) };
+static struct rb_mjit_unit_list compact_units;
 /* The number of so far processed ISEQs, used to generate unique id.  */
 static int current_unit_num;
 /* A mutex for conitionals and critical sections.  */
@@ -312,20 +318,57 @@ mjit_warning(const char *format, ...)
     }
 }
 
+/* Allocate struct rb_mjit_unit_node and return it. This MUST NOT be
+   called inside critical section because that causes deadlock. ZALLOC
+   may fire GC and GC hooks mjit_gc_start_hook that starts critical section. */
+static struct rb_mjit_unit_node *
+create_list_node(struct rb_mjit_unit *unit)
+{
+    struct rb_mjit_unit_node *node = calloc(1, sizeof(struct rb_mjit_unit_node)); /* To prevent GC, don't use ZALLOC */
+    if (node == NULL) return NULL;
+    node->unit = unit;
+    return node;
+}
+
 /* Add unit node to the tail of doubly linked LIST.  It should be not in
    the list before.  */
 static void
-add_to_list(struct rb_mjit_unit *unit, struct rb_mjit_unit_list *list)
+add_to_list(struct rb_mjit_unit_node *node, struct rb_mjit_unit_list *list)
 {
-    list_add_tail(&list->head, &unit->unode);
+    /* Append iseq to list */
+    if (list->head == NULL) {
+        list->head = node;
+    }
+    else {
+        struct rb_mjit_unit_node *tail = list->head;
+        while (tail->next != NULL) {
+            tail = tail->next;
+        }
+        tail->next = node;
+        node->prev = tail;
+    }
     list->length++;
 }
 
 static void
-remove_from_list(struct rb_mjit_unit *unit, struct rb_mjit_unit_list *list)
+remove_from_list(struct rb_mjit_unit_node *node, struct rb_mjit_unit_list *list)
 {
-    list_del(&unit->unode);
+    if (node->prev && node->next) {
+        node->prev->next = node->next;
+        node->next->prev = node->prev;
+    }
+    else if (node->prev == NULL && node->next) {
+        list->head = node->next;
+        node->next->prev = NULL;
+    }
+    else if (node->prev && node->next == NULL) {
+        node->prev->next = NULL;
+    }
+    else {
+        list->head = NULL;
+    }
     list->length--;
+    free(node);
 }
 
 static void
@@ -454,26 +497,28 @@ mjit_valid_class_serial_p(rb_serial_t class_serial)
 /* Return the best unit from list.  The best is the first
    high priority unit or the unit whose iseq has the biggest number
    of calls so far.  */
-static struct rb_mjit_unit *
+static struct rb_mjit_unit_node *
 get_from_list(struct rb_mjit_unit_list *list)
 {
-    struct rb_mjit_unit *unit = NULL, *next, *best = NULL;
+    struct rb_mjit_unit_node *node, *next, *best = NULL;
+
+    if (list->head == NULL)
+        return NULL;
 
     /* Find iseq with max total_calls */
-    list_for_each_safe(&list->head, unit, next, unode) {
-        if (unit->iseq == NULL) { /* ISeq is GCed. */
-            remove_from_list(unit, list);
-            free_unit(unit);
+    for (node = list->head; node != NULL; node = next) {
+        next = node->next;
+        if (node->unit->iseq == NULL) { /* ISeq is GCed. */
+            free_unit(node->unit);
+            remove_from_list(node, list);
             continue;
         }
 
-        if (best == NULL || best->iseq->body->total_calls < unit->iseq->body->total_calls) {
-            best = unit;
+        if (best == NULL || best->unit->iseq->body->total_calls < node->unit->iseq->body->total_calls) {
+            best = node;
         }
     }
-    if (best) {
-        remove_from_list(best, list);
-    }
+
     return best;
 }
 
@@ -836,7 +881,8 @@ static void
 compact_all_jit_code(void)
 {
 # ifndef _WIN32 /* This requires header transformation but we don't transform header on Windows for now */
-    struct rb_mjit_unit *unit, *cur = 0;
+    struct rb_mjit_unit *unit;
+    struct rb_mjit_unit_node *node;
     double start_time, end_time;
     static const char so_ext[] = DLEXT;
     char so_file[MAXPATHLEN];
@@ -853,8 +899,8 @@ compact_all_jit_code(void)
     o_files = alloca(sizeof(char *) * (active_units.length + 1));
     o_files[active_units.length] = NULL;
     CRITICAL_SECTION_START(3, "in compact_all_jit_code to keep .o files");
-    list_for_each(&active_units.head, cur, unode) {
-        o_files[i] = cur->o_file;
+    for (node = active_units.head; node != NULL; node = node->next) {
+        o_files[i] = node->unit->o_file;
         i++;
     }
 
@@ -878,25 +924,27 @@ compact_all_jit_code(void)
         unit->handle = handle;
 
         /* lazily dlclose handle (and .so file for win32) on `mjit_finish()`. */
-        add_to_list(unit, &compact_units);
+        node = calloc(1, sizeof(struct rb_mjit_unit_node)); /* To prevent GC, don't use ZALLOC */
+        node->unit = unit;
+        add_to_list(node, &compact_units);
 
         if (!mjit_opts.save_temps)
             remove_so_file(so_file, unit);
 
         CRITICAL_SECTION_START(3, "in compact_all_jit_code to read list");
-        list_for_each(&active_units.head, cur, unode) {
+        for (node = active_units.head; node != NULL; node = node->next) {
             void *func;
             char funcname[35]; /* TODO: reconsider `35` */
-            sprintf(funcname, "_mjit%d", cur->id);
+            sprintf(funcname, "_mjit%d", node->unit->id);
 
             if ((func = dlsym(handle, funcname)) == NULL) {
                 mjit_warning("skipping to reload '%s' from '%s': %s", funcname, so_file, dlerror());
                 continue;
             }
 
-            if (cur->iseq) { /* Check whether GCed or not */
+            if (node->unit->iseq) { /* Check whether GCed or not */
                 /* Usage of jit_code might be not in a critical section.  */
-                MJIT_ATOMIC_SET(cur->iseq->body->jit_func, (mjit_func_t)func);
+                MJIT_ATOMIC_SET(node->unit->iseq->body->jit_func, (mjit_func_t)func);
             }
         }
         CRITICAL_SECTION_FINISH(3, "in compact_all_jit_code to read list");
@@ -1041,13 +1089,6 @@ convert_unit_to_func(struct rb_mjit_unit *unit, struct rb_call_cache *cc_entries
     in_jit = TRUE;
     CRITICAL_SECTION_FINISH(3, "before mjit_compile to wait GC finish");
 
-    /* We need to check again here because we could've waited on GC above */
-    if (unit->iseq == NULL) {
-        if (!mjit_opts.save_temps)
-            remove_file(c_file);
-        free_unit(unit);
-        return (mjit_func_t)NOT_COMPILED_JIT_ISEQ_FUNC;
-    }
     {
         VALUE s = rb_iseq_path(unit->iseq);
         const char *label = RSTRING_PTR(unit->iseq->body->location.label);
@@ -1105,8 +1146,14 @@ convert_unit_to_func(struct rb_mjit_unit *unit, struct rb_call_cache *cc_entries
         remove_so_file(so_file, unit);
 
     if ((uintptr_t)func > (uintptr_t)LAST_JIT_ISEQ_FUNC) {
+        struct rb_mjit_unit_node *node = create_list_node(unit);
+        if (node == NULL) {
+            mjit_warning("failed to allocate a node to be added to active_units");
+            return (mjit_func_t)NOT_COMPILED_JIT_ISEQ_FUNC;
+        }
+
         CRITICAL_SECTION_START(3, "end of jit");
-        add_to_list(unit, &active_units);
+        add_to_list(node, &active_units);
         if (unit->iseq)
             print_jit_result("success", unit, end_time - start_time, c_file);
         CRITICAL_SECTION_FINISH(3, "end of jit");
@@ -1169,22 +1216,22 @@ mjit_worker(void)
 
     /* main worker loop */
     while (!stop_worker_p) {
-        struct rb_mjit_unit *unit;
+        struct rb_mjit_unit_node *node;
 
         /* wait until unit is available */
         CRITICAL_SECTION_START(3, "in worker dequeue");
-        while ((list_empty(&unit_queue.head) || active_units.length >= mjit_opts.max_cache_size) && !stop_worker_p) {
+        while ((unit_queue.head == NULL || active_units.length >= mjit_opts.max_cache_size) && !stop_worker_p) {
             rb_native_cond_wait(&mjit_worker_wakeup, &mjit_engine_mutex);
             verbose(3, "Getting wakeup from client");
         }
-        unit = get_from_list(&unit_queue);
+        node = get_from_list(&unit_queue);
         CRITICAL_SECTION_FINISH(3, "in worker dequeue");
 
-        if (unit) {
+        if (node) {
             mjit_func_t func;
             struct mjit_copy_job job;
 
-            job.body = unit->iseq->body;
+            job.body = node->unit->iseq->body;
             job.cc_entries = NULL;
             if (job.body->ci_size > 0 || job.body->ci_kw_size > 0)
                 job.cc_entries = alloca(sizeof(struct rb_call_cache) * (job.body->ci_size + job.body->ci_kw_size));
@@ -1203,13 +1250,14 @@ mjit_worker(void)
             }
 
             /* JIT compile */
-            func = convert_unit_to_func(unit, job.cc_entries, job.is_entries);
+            func = convert_unit_to_func(node->unit, job.cc_entries, job.is_entries);
 
             CRITICAL_SECTION_START(3, "in jit func replace");
-            if (unit->iseq) { /* Check whether GCed or not */
+            if (node->unit->iseq) { /* Check whether GCed or not */
                 /* Usage of jit_code might be not in a critical section.  */
-                MJIT_ATOMIC_SET(unit->iseq->body->jit_func, func);
+                MJIT_ATOMIC_SET(node->unit->iseq->body->jit_func, func);
             }
+            remove_from_list(node, &unit_queue);
             CRITICAL_SECTION_FINISH(3, "in jit func replace");
 
 #ifndef _MSC_VER