1
Fork 0
mirror of git://git.sv.gnu.org/emacs.git synced 2026-04-28 01:00:52 -07:00

Backport: Fix memory leak for global module objects (Bug#42482).

Instead of storing the global values in a global 'emacs_value_storage'
object, store them as hash values alongside the reference counts.
That way the garbage collector takes care of cleaning them up.

* src/emacs-module.c (global_storage): Remove.
(struct module_global_reference): New pseudovector type.
(XMODULE_GLOBAL_REFERENCE): New helper function.
(module_make_global_ref, module_free_global_ref): Use
'module_global_reference' struct for global reference values.
(value_to_lisp, module_handle_nonlocal_exit): Adapt to deletion of
'global_storage'.

(cherry picked from commit 5c5eb97908)
This commit is contained in:
Philipp Stephani 2020-07-23 13:48:43 +02:00
parent e12d1fbc15
commit 8ecca2f09f

View file

@ -159,11 +159,11 @@ struct emacs_value_frame
/* A structure that holds an initial frame (so that the first local /* A structure that holds an initial frame (so that the first local
values require no dynamic allocation) and keeps track of the values require no dynamic allocation) and keeps track of the
current frame. */ current frame. */
static struct emacs_value_storage struct emacs_value_storage
{ {
struct emacs_value_frame initial; struct emacs_value_frame initial;
struct emacs_value_frame *current; struct emacs_value_frame *current;
} global_storage; };
/* Private runtime and environment members. */ /* Private runtime and environment members. */
@ -351,10 +351,35 @@ module_get_environment (struct emacs_runtime *ert)
} }
/* To make global refs (GC-protected global values) keep a hash that /* To make global refs (GC-protected global values) keep a hash that
maps global Lisp objects to reference counts. */ maps global Lisp objects to 'struct module_global_reference'
objects. We store the 'emacs_value' in the hash table so that it
is automatically garbage-collected (Bug#42482). */
static Lisp_Object Vmodule_refs_hash; static Lisp_Object Vmodule_refs_hash;
/* Pseudovector type for global references. The pseudovector tag is
PVEC_OTHER since these values are never printed and don't need to
be special-cased for garbage collection. */
struct module_global_reference {
/* Pseudovector header, must come first. */
union vectorlike_header header;
/* Holds the emacs_value for the object. The Lisp_Object stored
therein must be the same as the hash key. */
struct emacs_value_tag value;
/* Reference count, always positive. */
ptrdiff_t refcount;
};
static struct module_global_reference *
XMODULE_GLOBAL_REFERENCE (Lisp_Object o)
{
eassert (PSEUDOVECTORP (o, PVEC_OTHER));
return XUNTAG (o, Lisp_Vectorlike, struct module_global_reference);
}
static emacs_value static emacs_value
module_make_global_ref (emacs_env *env, emacs_value ref) module_make_global_ref (emacs_env *env, emacs_value ref)
{ {
@ -363,21 +388,30 @@ module_make_global_ref (emacs_env *env, emacs_value ref)
Lisp_Object new_obj = value_to_lisp (ref), hashcode; Lisp_Object new_obj = value_to_lisp (ref), hashcode;
ptrdiff_t i = hash_lookup (h, new_obj, &hashcode); ptrdiff_t i = hash_lookup (h, new_obj, &hashcode);
/* Note: This approach requires the garbage collector to never move
objects. */
if (i >= 0) if (i >= 0)
{ {
Lisp_Object value = HASH_VALUE (h, i); Lisp_Object value = HASH_VALUE (h, i);
EMACS_INT refcount = XFIXNAT (value) + 1; struct module_global_reference *ref = XMODULE_GLOBAL_REFERENCE (value);
if (MOST_POSITIVE_FIXNUM < refcount) bool overflow = INT_ADD_WRAPV (ref->refcount, 1, &ref->refcount);
if (overflow)
overflow_error (); overflow_error ();
value = make_fixed_natnum (refcount); return &ref->value;
set_hash_value_slot (h, i, value);
} }
else else
{ {
hash_put (h, new_obj, make_fixed_natnum (1), hashcode); struct module_global_reference *ref
= ALLOCATE_PLAIN_PSEUDOVECTOR (struct module_global_reference,
PVEC_OTHER);
ref->value.v = new_obj;
ref->refcount = 1;
Lisp_Object value;
XSETPSEUDOVECTOR (value, ref, PVEC_OTHER);
hash_put (h, new_obj, value, hashcode);
return &ref->value;
} }
return allocate_emacs_value (env, &global_storage, new_obj);
} }
static void static void
@ -393,23 +427,16 @@ module_free_global_ref (emacs_env *env, emacs_value ref)
if (i >= 0) if (i >= 0)
{ {
EMACS_INT refcount = XFIXNAT (HASH_VALUE (h, i)) - 1; Lisp_Object value = HASH_VALUE (h, i);
if (refcount > 0) struct module_global_reference *ref = XMODULE_GLOBAL_REFERENCE (value);
set_hash_value_slot (h, i, make_fixed_natnum (refcount)); eassert (0 < ref->refcount);
else if (--ref->refcount == 0)
{ hash_remove_from_table (h, obj);
eassert (refcount == 0);
hash_remove_from_table (h, obj);
}
} }
else if (module_assertions)
if (module_assertions)
{ {
ptrdiff_t count = 0;
if (value_storage_contains_p (&global_storage, ref, &count))
return;
module_abort ("Global value was not found in list of %"pD"d globals", module_abort ("Global value was not found in list of %"pD"d globals",
count); h->count);
} }
} }
@ -1190,8 +1217,10 @@ value_to_lisp (emacs_value v)
++num_environments; ++num_environments;
} }
/* Also check global values. */ /* Also check global values. */
if (value_storage_contains_p (&global_storage, v, &num_values)) struct Lisp_Hash_Table *h = XHASH_TABLE (Vmodule_refs_hash);
if (hash_lookup (h, v->v, NULL) != -1)
goto ok; goto ok;
INT_ADD_WRAPV (num_values, h->count, &num_values);
module_abort (("Emacs value not found in %"pD"d values " module_abort (("Emacs value not found in %"pD"d values "
"of %"pD"d environments"), "of %"pD"d environments"),
num_values, num_environments); num_values, num_environments);
@ -1404,10 +1433,7 @@ module_handle_nonlocal_exit (emacs_env *env, enum nonlocal_exit type,
void void
init_module_assertions (bool enable) init_module_assertions (bool enable)
{ {
/* If enabling module assertions, use a hidden environment for
storing the globals. This environment is never freed. */
module_assertions = enable; module_assertions = enable;
initialize_storage (&global_storage);
} }
/* Return whether STORAGE contains VALUE. Used to check module /* Return whether STORAGE contains VALUE. Used to check module