NativeCallback hash is now a hash of lists.

Generating multiple closures of the same code object
would confuse NativeCall and only one of the callbacks
would ever be reached. Now we compare addresses as well
and store and mark all callbacks to all closure-cloned
versions of the same MVMCode.

src/core/nativecall.c

@@ -286,8 +286,10 @@ static void * unmarshal_carray(MVMThreadContext *tc, MVMObject *value) {
 /* Sets up a callback, caching the information to avoid duplicate work. */
 static char callback_handler(DCCallback *cb, DCArgs *args, DCValue *result, MVMNativeCallback *data);
 static void * unmarshal_callback(MVMThreadContext *tc, MVMObject *callback, MVMObject *sig_info) {
-    MVMNativeCallback *callback_data;
-    MVMString         *cuid;
+    MVMNativeCallbackCacheHead *callback_data_head = NULL;
+    MVMNativeCallback **callback_data_handle = NULL;
+    MVMString          *cuid;
+    MVMuint8            found = 0;
 
     if (!IS_CONCRETE(callback))
         return NULL;
@@ -296,20 +298,38 @@ static void * unmarshal_callback(MVMThreadContext *tc, MVMObject *callback, MVMO
     callback = MVM_frame_find_invokee(tc, callback, NULL);
     cuid     = ((MVMCode *)callback)->body.sf->body.cuuid;
     MVM_string_flatten(tc, cuid);
-    MVM_HASH_GET(tc, tc->native_callback_cache, cuid, callback_data);
+    MVM_HASH_GET(tc, tc->native_callback_cache, cuid, callback_data_head);
 
-    if (!callback_data) {
-        /* Need to build/cache callback data entry. */
+    if (!callback_data_head) {
+        callback_data_head = malloc(sizeof(MVMNativeCallbackCacheHead));
+        callback_data_head->head = NULL;
+
+        MVM_HASH_BIND(tc, tc->native_callback_cache, cuid, callback_data_head);
+    }
+
+    callback_data_handle = &(callback_data_head->head);
+
+    while (*callback_data_handle && !found) {
+        if ((*callback_data_handle)->target == callback) {
+            found = 1;
+        } else {
+            callback_data_handle = &((*callback_data_handle)->next);
+        }
+    }
+    if (!*callback_data_handle) {
+        /* First, build the MVMNativeCallback */
         MVMCallsite *cs;
         char        *signature;
         MVMObject   *typehash;
         MVMint64     num_info, i;
+        MVMNativeCallback *callback_data;
 
         num_info                 = MVM_repr_elems(tc, sig_info);
         callback_data            = malloc(sizeof(MVMNativeCallback));
         callback_data->num_types = num_info;
         callback_data->typeinfos = malloc(num_info * sizeof(MVMint16));
         callback_data->types     = malloc(num_info * sizeof(MVMObject *));
+        callback_data->next      = NULL;
 
         /* A dyncall signature looks like this: xxx)x
         * Argument types before the ) and return type after it. Thus,
@@ -363,10 +383,11 @@ static void * unmarshal_callback(MVMThreadContext *tc, MVMObject *callback, MVMO
         callback_data->target    = callback;
         callback_data->cb        = dcbNewCallback(signature, &callback_handler, callback_data);
 
-        MVM_HASH_BIND(tc, tc->native_callback_cache, cuid, callback_data);
+        /* Now insert the MVMCallback into the linked list. */
+        *callback_data_handle = callback_data;
     }
 
-    return callback_data->cb;
+    return (*callback_data_handle)->cb;
 }
 
 /* Called to handle a callback. */

src/core/nativecall.h

@@ -21,9 +21,10 @@
 #define MVM_NATIVECALL_ARG_FREE_STR        1
 #define MVM_NATIVECALL_ARG_FREE_STR_MASK   1
 
-/* Native callback entry. Hung off ThreadContext, keyed on CUID of code ref.
- * (May be better off a CompUnit, though that may cause an edge case where a
- * premature collection is possible.) */
+/* Native callback entry. Hung off MVMNativeCallbackCacheHead, which is
+ * a hash owned by the ThreadContext. All MVMNativeCallbacks in a linked
+ * list have the same cuid, which is the key to the CacheHead hash.
+ */
 struct MVMNativeCallback {
     /* The dyncall callback object. */
     DCCallback *cb;
@@ -46,6 +47,19 @@ struct MVMNativeCallback {
     /* The MoarVM callsite object for this call. */
     MVMCallsite *cs;
 
+    /* The next entry in the linked list */
+    MVMNativeCallback *next;
+};
+
+
+/* A hash of nativecall callbacks. Each entry is a linked
+ * list of MVMNativeCallback sharing the same cuid.
+ * Multiple callbacks with the same cuid get created when
+ * closures are taken and need to be differentiated.
+ */
+struct MVMNativeCallbackCacheHead {
+    MVMNativeCallback *head;
+
     /* The uthash hash handle inline struct. */
     UT_hash_handle hash_handle;
 };

src/core/threadcontext.h

@@ -176,7 +176,7 @@ struct MVMThreadContext {
     MVMObject     *cur_dispatcher;
 
     /* Cache of native code callback data. */
-    MVMNativeCallback *native_callback_cache;
+    MVMNativeCallbackCacheHead *native_callback_cache;
 
     /* Random number generator state. */
     MVMuint64 rand_state[2];

src/gc/roots.c

@@ -65,7 +65,7 @@ void MVM_gc_root_add_instance_roots_to_worklist(MVMThreadContext *tc, MVMGCWorkl
 /* Adds anything that is a root thanks to being referenced by a thread,
  * context, but that isn't permanent. */
 void MVM_gc_root_add_tc_roots_to_worklist(MVMThreadContext *tc, MVMGCWorklist *worklist) {
-    MVMNativeCallback *current_cbce, *tmp_cbce;
+    MVMNativeCallbackCacheHead *current_cbceh, *tmp_cbceh;
 
     /* Any active exception handlers. */
     MVMActiveHandler *cur_ah = tc->active_handlers;
@@ -91,11 +91,15 @@ void MVM_gc_root_add_tc_roots_to_worklist(MVMThreadContext *tc, MVMGCWorklist *w
     MVM_gc_worklist_add(tc, worklist, &tc->cur_dispatcher);
 
     /* Callback cache. */
-    HASH_ITER(hash_handle, tc->native_callback_cache, current_cbce, tmp_cbce) {
+    HASH_ITER(hash_handle, tc->native_callback_cache, current_cbceh, tmp_cbceh) {
         MVMint32 i;
-        for (i = 0; i < current_cbce->num_types; i++)
-            MVM_gc_worklist_add(tc, worklist, &current_cbce->types[i]);
-        MVM_gc_worklist_add(tc, worklist, &current_cbce->target);
+        MVMNativeCallback *entry = current_cbceh->head;
+        while (entry) {
+            for (i = 0; i < entry->num_types; i++)
+                MVM_gc_worklist_add(tc, worklist, &(entry->types[i]));
+            MVM_gc_worklist_add(tc, worklist, &(entry->target));
+            entry = entry->next;
+        }
     }
 }
 

src/types.h

@@ -172,3 +172,4 @@ typedef struct MVMDecodeStream MVMDecodeStream;
 typedef struct MVMDecodeStreamBytes MVMDecodeStreamBytes;
 typedef struct MVMDecodeStreamChars MVMDecodeStreamChars;
 typedef struct MVMNativeCallback MVMNativeCallback;
+typedef struct MVMNativeCallbackCacheHead MVMNativeCallbackCacheHead;