[erlang-questions] Closures across module reloads, undocumented behavior?

[Matthias Lang]

On Thursday, July 29, Nahuel Greco wrote:
> I found an strange behavior, probably undocumented:

I think this is the same bug as reported here:

  http://erlang.org/pipermail/erlang-bugs/2007-June/000368.html

The root cause is that a fun with the same hash as an existing fun
will always overwrite the existing fun on code load. I.e. the "is the
hash the same?" test is too weak.

I provided a patch a few years ago, but it was never incorporated. The
patch itself has also been stripped from the mailing list archive.

  http://www.erlang.org/cgi-bin/ezmlm-cgi?4:mss:32340:200801:aifmekcigbbmiidkfbgo

Patch is appended again below, it was originally written for R12B, but
applies cleanly to R14A and (brief check) seems to work.

Does the output below from the version with the patch correspond to
what you expect?

Matthias

----------------------------------------------------------------------
Your test case running on the official R14A:

- Unloading m old and current 
- Loading m__base and starting process 
Started a() from version 1 
>From closure, version() -> 1, B -> first 
- Loaded m__2 , differences with base: Increments the version 
- Continuing process 
>From closure, version() -> 2, B -> first 

- Unloading m old and current 
- Loading m__base and starting process 
Started a() from version 1 
>From closure, version() -> 1, B -> first 
- Loaded m__3 , differences with base: Increments the version, version/0 changed to another_version/0 
- Continuing process 
>From closure, version() -> 1, B -> first 

- Unloading m old and current 
- Loading m__base and starting process 
Started a() from version 1 
>From closure, version() -> 1, B -> first 
- Loaded m__4 , differences with base: Increments the version, changed text literal in closure 
- Continuing process 
>From closure, version() -> 1, B -> first 

- Unloading m old and current 
- Loading m__base and starting process 
Started a() from version 1 
>From closure, version() -> 1, B -> first 
- Loaded m__5 , differences with base: Increments the version, changes B assigned atom to 'second' 
- Continuing process 
>From closure, version() -> 2, B -> second 
[undef,undef,undef,undef]

----------------------------------------------------------------------
Your test case running on R14A with my patch

- Unloading m old and current 
- Loading m__base and starting process 
Started a() from version 1 
>From closure, version() -> 1, B -> first 
- Loaded m__2 , differences with base: Increments the version 
- Continuing process 
>From closure, version() -> 1, B -> first 

- Unloading m old and current 
- Loading m__base and starting process 
Started a() from version 1 
>From closure, version() -> 1, B -> first 
- Loaded m__3 , differences with base: Increments the version, version/0 changed to another_version/0 
- Continuing process 
>From closure, version() -> 1, B -> first 

- Unloading m old and current 
- Loading m__base and starting process 
Started a() from version 1 
>From closure, version() -> 1, B -> first 
- Loaded m__4 , differences with base: Increments the version, changed text literal in closure 
- Continuing process 
>From closure, version() -> 1, B -> first 

- Unloading m old and current 
- Loading m__base and starting process 
Started a() from version 1 
>From closure, version() -> 1, B -> first 
- Loaded m__5 , differences with base: Increments the version, changes B assigned atom to 'second' 
- Continuing process 
>From closure, version() -> 1, B -> first 
[undef,undef,undef,undef]

----------------------------------------------------------------------
The patch:

--- erl_fun.c.orig	2008-01-17 00:03:10.000000000 +0100
+++ erl_fun.c	2008-01-17 00:23:23.000000000 +0100
@@ -97,6 +97,7 @@
     long refc;
     ASSERT(is_atom(mod));
     template.old_uniq = uniq;
+    sys_memset(template.uniq, 0, sizeof(template.uniq));
     template.old_index = index;
     template.module = mod;
     erts_fun_write_lock();
@@ -120,6 +121,7 @@
 
     ASSERT(is_atom(mod));
     template.old_uniq = old_uniq;
+    sys_memcpy(template.uniq, uniq, sizeof(template.uniq));
     template.old_index = old_index;
     template.module = mod;
     erts_fun_write_lock();
@@ -279,7 +281,10 @@
 static HashValue
 fun_hash(ErlFunEntry* obj)
 {
-    return (HashValue) (obj->old_uniq ^ obj->old_index ^ atom_val(obj->module));
+    return (HashValue) (obj->old_uniq ^ 
+                        *(int*)obj->uniq ^ 
+                        obj->old_index ^ 
+                        atom_val(obj->module));
 }
 
 static int
@@ -287,6 +292,7 @@
 {
     return !(obj1->module == obj2->module && 
 	     obj1->old_uniq == obj2->old_uniq &&
+             sys_memcmp(obj1->uniq, obj2->uniq, sizeof(obj1->uniq)) == 0 &&
 	     obj1->old_index == obj2->old_index);
 }
 
@@ -297,6 +303,7 @@
 						  sizeof(ErlFunEntry));
 
     obj->old_uniq = template->old_uniq;
+    sys_memcpy(obj->uniq, template->uniq, sizeof(template->uniq));
     obj->old_index = template->old_index;
     obj->module = template->module;
     erts_refc_init(&obj->refc, -1);