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

[Nahuel Greco]

I think you are right, seems to be the same bug. I applied your patch
to R14A and got the same results. Also, after patching, I tested if
code:purge/1 kills the processes having a closure referencing the old
module version, and yes, they are killed.

I expected a consistent behaviour, so I think the patch is good for me
:) But the desired behaviour is at least undocumented, it needs to be
defined and/or documented officially.

Thanks.

Saludos,
Nahuel Greco.



On Thu, Aug 12, 2010 at 5:21 AM, Matthias Lang <matthias@REDACTED> wrote:
> 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);
>
>
>