GNU bug report logs - #42482
27.0.91; emacs modules memory leak

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 42482 in the body.
You can then email your comments to 42482 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Milan Stanojević <mstanojevic <at> janestreet.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.0.91; emacs modules memory leak
Date: Wed, 22 Jul 2020 19:25:19 -0400

[Message part 1 (text/plain, inline)]

env-make_global_ref adds a reference to the underlying Lisp_Object
and allocates emacs_value from the global storage. env->free_global_ref
on the other hand will only remove a reference to the underlying
Lisp_Object and not free the emacs_value.

Here is a simple recipe to reproduce the problem (I only tested this
on linux). I'm attaching the necessary files.

$ gcc -shared -fpic -std=c99 -I <dir-with-emacs-module.h>
create_global_refs.c -o create_global_refs.so
$ emacs --no-splash -q -l create_global_refs.so -l create_global_refs_test.el

If you look at the memory usage of emacs (for example in htop) you'll
see that with emacs-26 it is constant but with emacs-27 the resident
memory quickly grows.

[create_global_refs.c (text/x-csrc, attachment)]

[create_global_refs_test.el (text/x-emacs-lisp, attachment)]

Message #10 received at 42482-done <at> debbugs.gnu.org (full text, mbox):

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: Milan Stanojević <mstanojevic <at> janestreet.com>
Cc: 42482-done <at> debbugs.gnu.org
Subject: Re: bug#42482: 27.0.91; emacs modules memory leak
Date: Thu, 23 Jul 2020 14:06:57 +0200

Am Do., 23. Juli 2020 um 01:27 Uhr schrieb Milan Stanojević
<mstanojevic <at> janestreet.com>:
>
> env-make_global_ref adds a reference to the underlying Lisp_Object
> and allocates emacs_value from the global storage. env->free_global_ref
> on the other hand will only remove a reference to the underlying
> Lisp_Object and not free the emacs_value.
>
> Here is a simple recipe to reproduce the problem (I only tested this
> on linux). I'm attaching the necessary files.
>
> $ gcc -shared -fpic -std=c99 -I <dir-with-emacs-module.h>
> create_global_refs.c -o create_global_refs.so
> $ emacs --no-splash -q -l create_global_refs.so -l create_global_refs_test.el
>
> If you look at the memory usage of emacs (for example in htop) you'll
> see that with emacs-26 it is constant but with emacs-27 the resident
> memory quickly grows.

Thanks for the report. I've fixed this in commit
5c5eb9790898e4ab10bcbbdb6871947ed3018569; the fix is slightly
different from what you proposed in that it stores the emacs_value
object in the global references hashtable, but it should have the same
effect. At least I can't reproduce the symptom any more after that
commit.

Message #13 received at 42482-done <at> debbugs.gnu.org (full text, mbox):

From: Milan Stanojević <mstanojevic <at> janestreet.com>
To: Philipp Stephani <p.stephani2 <at> gmail.com>
Cc: 42482-done <at> debbugs.gnu.org
Subject: Re: bug#42482: 27.0.91; emacs modules memory leak
Date: Thu, 23 Jul 2020 10:29:08 -0400

Thank you for the quick fix.
Is there a chance this also goes to emacs-27 branch so it can be in
the emacs 27.1 when it gets released?

On Thu, Jul 23, 2020 at 8:07 AM Philipp Stephani <p.stephani2 <at> gmail.com> wrote:
>
> Am Do., 23. Juli 2020 um 01:27 Uhr schrieb Milan Stanojević
> <mstanojevic <at> janestreet.com>:
> >
> > env-make_global_ref adds a reference to the underlying Lisp_Object
> > and allocates emacs_value from the global storage. env->free_global_ref
> > on the other hand will only remove a reference to the underlying
> > Lisp_Object and not free the emacs_value.
> >
> > Here is a simple recipe to reproduce the problem (I only tested this
> > on linux). I'm attaching the necessary files.
> >
> > $ gcc -shared -fpic -std=c99 -I <dir-with-emacs-module.h>
> > create_global_refs.c -o create_global_refs.so
> > $ emacs --no-splash -q -l create_global_refs.so -l create_global_refs_test.el
> >
> > If you look at the memory usage of emacs (for example in htop) you'll
> > see that with emacs-26 it is constant but with emacs-27 the resident
> > memory quickly grows.
>
> Thanks for the report. I've fixed this in commit
> 5c5eb9790898e4ab10bcbbdb6871947ed3018569; the fix is slightly
> different from what you proposed in that it stores the emacs_value
> object in the global references hashtable, but it should have the same
> effect. At least I can't reproduce the symptom any more after that
> commit.

Message #16 received at 42482-done <at> debbugs.gnu.org (full text, mbox):

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: Milan Stanojević <mstanojevic <at> janestreet.com>
Cc: 42482-done <at> debbugs.gnu.org
Subject: Re: bug#42482: 27.0.91; emacs modules memory leak
Date: Thu, 23 Jul 2020 16:33:12 +0200

Am Do., 23. Juli 2020 um 16:29 Uhr schrieb Milan Stanojević
<mstanojevic <at> janestreet.com>:
>
> Thank you for the quick fix.
> Is there a chance this also goes to emacs-27 branch so it can be in
> the emacs 27.1 when it gets released?

I think backporting the fix should be fine, as the fix is rather
localized and fixes a regression. Eli?

>
> On Thu, Jul 23, 2020 at 8:07 AM Philipp Stephani <p.stephani2 <at> gmail.com> wrote:
> >
> > Am Do., 23. Juli 2020 um 01:27 Uhr schrieb Milan Stanojević
> > <mstanojevic <at> janestreet.com>:
> > >
> > > env-make_global_ref adds a reference to the underlying Lisp_Object
> > > and allocates emacs_value from the global storage. env->free_global_ref
> > > on the other hand will only remove a reference to the underlying
> > > Lisp_Object and not free the emacs_value.
> > >
> > > Here is a simple recipe to reproduce the problem (I only tested this
> > > on linux). I'm attaching the necessary files.
> > >
> > > $ gcc -shared -fpic -std=c99 -I <dir-with-emacs-module.h>
> > > create_global_refs.c -o create_global_refs.so
> > > $ emacs --no-splash -q -l create_global_refs.so -l create_global_refs_test.el
> > >
> > > If you look at the memory usage of emacs (for example in htop) you'll
> > > see that with emacs-26 it is constant but with emacs-27 the resident
> > > memory quickly grows.
> >
> > Thanks for the report. I've fixed this in commit
> > 5c5eb9790898e4ab10bcbbdb6871947ed3018569; the fix is slightly
> > different from what you proposed in that it stores the emacs_value
> > object in the global references hashtable, but it should have the same
> > effect. At least I can't reproduce the symptom any more after that
> > commit.

Message #19 received at 42482 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philipp Stephani <p.stephani2 <at> gmail.com>
Cc: 42482 <at> debbugs.gnu.org, mstanojevic <at> janestreet.com
Subject: Re: bug#42482: 27.0.91; emacs modules memory leak
Date: Thu, 23 Jul 2020 20:45:28 +0300

> From: Philipp Stephani <p.stephani2 <at> gmail.com>
> Date: Thu, 23 Jul 2020 16:33:12 +0200
> Cc: 42482-done <at> debbugs.gnu.org
> 
> Am Do., 23. Juli 2020 um 16:29 Uhr schrieb Milan Stanojević
> <mstanojevic <at> janestreet.com>:
> >
> > Thank you for the quick fix.
> > Is there a chance this also goes to emacs-27 branch so it can be in
> > the emacs 27.1 when it gets released?
> 
> I think backporting the fix should be fine, as the fix is rather
> localized and fixes a regression. Eli?

How well was it tested?  The change is not exactly trivial.  But if
you are satisfied with the testing enough to have this in emacs-27,
I'm okay with that.

Thanks.

Message #22 received at 42482 <at> debbugs.gnu.org (full text, mbox):

From: Milan Stanojević <mstanojevic <at> janestreet.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Philipp Stephani <p.stephani2 <at> gmail.com>, 42482 <at> debbugs.gnu.org
Subject: Re: bug#42482: 27.0.91; emacs modules memory leak
Date: Thu, 23 Jul 2020 15:11:31 -0400

I backported the change to emacs-27 branch locally and tried a simple
test using our real-world emacs module against it and the change seems
to fix the memory leak. Our module (and the test I tried) is written
in ocaml (calls to/from emacs are via C bindings) and relies on
interplay between ocaml and emacs gcs to not leak memory, so that
working is a good sign.

Compared to emacs-26, it uses a bit more memory, but I think that is
expected. The main thing is that memory is stable.









On Thu, Jul 23, 2020 at 1:45 PM Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> > From: Philipp Stephani <p.stephani2 <at> gmail.com>
> > Date: Thu, 23 Jul 2020 16:33:12 +0200
> > Cc: 42482-done <at> debbugs.gnu.org
> >
> > Am Do., 23. Juli 2020 um 16:29 Uhr schrieb Milan Stanojević
> > <mstanojevic <at> janestreet.com>:
> > >
> > > Thank you for the quick fix.
> > > Is there a chance this also goes to emacs-27 branch so it can be in
> > > the emacs 27.1 when it gets released?
> >
> > I think backporting the fix should be fine, as the fix is rather
> > localized and fixes a regression. Eli?
>
> How well was it tested?  The change is not exactly trivial.  But if
> you are satisfied with the testing enough to have this in emacs-27,
> I'm okay with that.
>
> Thanks.

Message #25 received at 42482 <at> debbugs.gnu.org (full text, mbox):

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 42482 <at> debbugs.gnu.org,
 Milan Stanojević <mstanojevic <at> janestreet.com>
Subject: Re: bug#42482: 27.0.91; emacs modules memory leak
Date: Sat, 25 Jul 2020 23:43:13 +0200

Am Do., 23. Juli 2020 um 19:45 Uhr schrieb Eli Zaretskii <eliz <at> gnu.org>:
>
> > From: Philipp Stephani <p.stephani2 <at> gmail.com>
> > Date: Thu, 23 Jul 2020 16:33:12 +0200
> > Cc: 42482-done <at> debbugs.gnu.org
> >
> > Am Do., 23. Juli 2020 um 16:29 Uhr schrieb Milan Stanojević
> > <mstanojevic <at> janestreet.com>:
> > >
> > > Thank you for the quick fix.
> > > Is there a chance this also goes to emacs-27 branch so it can be in
> > > the emacs 27.1 when it gets released?
> >
> > I think backporting the fix should be fine, as the fix is rather
> > localized and fixes a regression. Eli?
>
> How well was it tested?  The change is not exactly trivial.  But if
> you are satisfied with the testing enough to have this in emacs-27,
> I'm okay with that.

I'd like to have a few more test cases around global references in
emacs-27, as the current test cases only test some simple/success
cases, and we'd probably want to test at least a few more edge cases
(e.g. freeing global references in a different order than allocating
them). I've added two more test cases on master and will see that I
can add a few more in the coming days.

Message #28 received at 42482 <at> debbugs.gnu.org (full text, mbox):

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 42482 <at> debbugs.gnu.org,
 Milan Stanojević <mstanojevic <at> janestreet.com>
Subject: Re: bug#42482: 27.0.91; emacs modules memory leak
Date: Sat, 1 Aug 2020 14:11:54 +0200

Am Sa., 25. Juli 2020 um 23:43 Uhr schrieb Philipp Stephani
<p.stephani2 <at> gmail.com>:
>
> Am Do., 23. Juli 2020 um 19:45 Uhr schrieb Eli Zaretskii <eliz <at> gnu.org>:
> >
> > > From: Philipp Stephani <p.stephani2 <at> gmail.com>
> > > Date: Thu, 23 Jul 2020 16:33:12 +0200
> > > Cc: 42482-done <at> debbugs.gnu.org
> > >
> > > Am Do., 23. Juli 2020 um 16:29 Uhr schrieb Milan Stanojević
> > > <mstanojevic <at> janestreet.com>:
> > > >
> > > > Thank you for the quick fix.
> > > > Is there a chance this also goes to emacs-27 branch so it can be in
> > > > the emacs 27.1 when it gets released?
> > >
> > > I think backporting the fix should be fine, as the fix is rather
> > > localized and fixes a regression. Eli?
> >
> > How well was it tested?  The change is not exactly trivial.  But if
> > you are satisfied with the testing enough to have this in emacs-27,
> > I'm okay with that.
>
> I'd like to have a few more test cases around global references in
> emacs-27, as the current test cases only test some simple/success
> cases, and we'd probably want to test at least a few more edge cases
> (e.g. freeing global references in a different order than allocating
> them). I've added two more test cases on master and will see that I
> can add a few more in the coming days.

I've now added a few more tests and backported the fix to the release branch.

Message #31 received at 42482 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philipp Stephani <p.stephani2 <at> gmail.com>
Cc: 42482 <at> debbugs.gnu.org, mstanojevic <at> janestreet.com
Subject: Re: bug#42482: 27.0.91; emacs modules memory leak
Date: Sat, 01 Aug 2020 15:37:40 +0300

> From: Philipp Stephani <p.stephani2 <at> gmail.com>
> Date: Sat, 1 Aug 2020 14:11:54 +0200
> Cc: Milan Stanojević <mstanojevic <at> janestreet.com>, 
> 	42482 <at> debbugs.gnu.org
> 
> I've now added a few more tests and backported the fix to the release branch.

I think it's too late for Emacs 27.1, sorry.  The tarball is already
made.

Message #34 received at 42482 <at> debbugs.gnu.org (full text, mbox):

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: Nicolas Petton <nico <at> petton.fr>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 42482 <at> debbugs.gnu.org,
 Milan Stanojević <mstanojevic <at> janestreet.com>
Subject: Re: bug#42482: 27.0.91; emacs modules memory leak
Date: Sat, 1 Aug 2020 16:47:18 +0200

Am Sa., 1. Aug. 2020 um 14:55 Uhr schrieb Nicolas Petton <nico <at> petton.fr>:
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> >> I've now added a few more tests and backported the fix to the release branch.
> >
> > I think it's too late for Emacs 27.1, sorry.  The tarball is already
> > made.
>
> If the bugfix is important enough, I can build a new RC tomorrow and set
> the release date to the 8th of Aug.


I don't feel strongly either way. My expectation for global references
is that they should mainly be used for the module equivalent of defvar
and friends, in which case the bug isn't that severe because nobody
needs millions of defvars. OTOH, people might use global references
for other means, including allocating them dynamically, in which case
leaking them can become annoying.

Message #37 received at 42482 <at> debbugs.gnu.org (full text, mbox):

From: Nicolas Petton <nico <at> petton.fr>
To: Eli Zaretskii <eliz <at> gnu.org>, Philipp Stephani <p.stephani2 <at> gmail.com>
Cc: 42482 <at> debbugs.gnu.org, mstanojevic <at> janestreet.com
Subject: Re: bug#42482: 27.0.91; emacs modules memory leak
Date: Sat, 01 Aug 2020 14:55:33 +0200

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> I've now added a few more tests and backported the fix to the release branch.
>
> I think it's too late for Emacs 27.1, sorry.  The tarball is already
> made.

If the bugfix is important enough, I can build a new RC tomorrow and set
the release date to the 8th of Aug.

Nico

[signature.asc (application/pgp-signature, inline)]

Message #40 received at 42482 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Nicolas Petton <nico <at> petton.fr>
Cc: p.stephani2 <at> gmail.com, 42482 <at> debbugs.gnu.org, mstanojevic <at> janestreet.com
Subject: Re: bug#42482: 27.0.91; emacs modules memory leak
Date: Sun, 02 Aug 2020 20:25:49 +0300

> From: Nicolas Petton <nico <at> petton.fr>
> Cc: 42482 <at> debbugs.gnu.org, mstanojevic <at> janestreet.com
> Date: Sat, 01 Aug 2020 14:55:33 +0200
> 
> > I think it's too late for Emacs 27.1, sorry.  The tarball is already
> > made.
> 
> If the bugfix is important enough, I can build a new RC tomorrow and set
> the release date to the 8th of Aug.

Unfortunately, a much more serious bug was reported and fixed today,
so we will need a new RC from the latest emacs-27 branch.

Thanks.

Message #43 received at 42482 <at> debbugs.gnu.org (full text, mbox):

From: Milan Stanojević <mstanojevic <at> janestreet.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Philipp Stephani <p.stephani2 <at> gmail.com>, 42482 <at> debbugs.gnu.org,
 Nicolas Petton <nico <at> petton.fr>
Subject: Re: bug#42482: 27.0.91; emacs modules memory leak
Date: Mon, 3 Aug 2020 19:02:01 -0400

I guess this fix will be included now in 27.1 but I just wanted to say
that OCaml emacs module library (https://github.com/janestreet/ecaml)
uses global refs everywhere so the leak would be very noticeable (that
is how we found out in the first place).

I'm not sure this would have been enough to postpone the release date
since I don't think ecaml has that many users (We do have 500+ devs
using and relying on ecaml but it is really a single emacs+ecaml
installation and we can patch emacs sources or just wait for 27.2)



On Sun, Aug 2, 2020 at 1:26 PM Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> > From: Nicolas Petton <nico <at> petton.fr>
> > Cc: 42482 <at> debbugs.gnu.org, mstanojevic <at> janestreet.com
> > Date: Sat, 01 Aug 2020 14:55:33 +0200
> >
> > > I think it's too late for Emacs 27.1, sorry.  The tarball is already
> > > made.
> >
> > If the bugfix is important enough, I can build a new RC tomorrow and set
> > the release date to the 8th of Aug.
>
> Unfortunately, a much more serious bug was reported and fixed today,
> so we will need a new RC from the latest emacs-27 branch.
>
> Thanks.

Message #46 received at 42482 <at> debbugs.gnu.org (full text, mbox):

From: Nicolas Petton <nico <at> petton.fr>
To: Eli Zaretskii <eliz <at> gnu.org>, Nicolas Petton <nico <at> petton.fr>
Cc: p.stephani2 <at> gmail.com, 42482 <at> debbugs.gnu.org, mstanojevic <at> janestreet.com
Subject: Re: bug#42482: 27.0.91; emacs modules memory leak
Date: Tue, 04 Aug 2020 10:05:38 +0200

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> If the bugfix is important enough, I can build a new RC tomorrow and set
>> the release date to the 8th of Aug.
>
> Unfortunately, a much more serious bug was reported and fixed today,
> so we will need a new RC from the latest emacs-27 branch.

That's ok :)

I'll make a new one this evening.

Nico

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.