unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#46916: 28.0.50; pure_alloc(10424, ...) fails badly when pure space is exhausted
@ 2021-03-04 12:24 Pip Cet
  2021-03-04 12:44 ` Pip Cet
  0 siblings, 1 reply; 10+ messages in thread
From: Pip Cet @ 2021-03-04 12:24 UTC (permalink / raw)
  To: 46916

Pure space should be removed. But until it is, it shouldn't exhaust
all virtual memory, crashing the machines of the unwary ones who tried
to build emacs without a ulimit...

The problem I ran into was that the native-comp branch calls Fpurecopy
on comp_u->data_vec. In my case, pure space was already exhausted and
data_vec had 1302 elements.

That caused purecopy() to call pure_alloc with a size argument of
10424. pure_alloc allocated another 10000 bytes of fake pure space,
found out those weren't enough, so it allocated another 10000 bytes of
fake pure space, which weren't enough, etc., until the process started
swapping.

I'm not sure who's the culprit here: the native-comp branch tries to
purecopy a large vector, which is a problem; purecopy() saw that
request and passed it on to pure_alloc(), rather than returning the
impure vector because it was too large. And pure_alloc() certainly
should fail more gracefully than it did (there's an eassert() to
prevent this situation, but that should be unconditionally compiled
instead).

I was going to write a message to emacs-devel (I'd started before
hitting this bug) proposing to remove pure-space now, but I think it's
fair to have a separate bug report if we decide to keep pure space
after all.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#46916: 28.0.50; pure_alloc(10424, ...) fails badly when pure space is exhausted
  2021-03-04 12:24 bug#46916: 28.0.50; pure_alloc(10424, ...) fails badly when pure space is exhausted Pip Cet
@ 2021-03-04 12:44 ` Pip Cet
  2021-03-04 14:15   ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Pip Cet @ 2021-03-04 12:44 UTC (permalink / raw)
  To: 46916

[-- Attachment #1: Type: text/plain, Size: 392 bytes --]

On Thu, Mar 4, 2021 at 12:26 PM Pip Cet <pipcet@gmail.com> wrote:
> Pure space should be removed. But until it is, it shouldn't exhaust
> all virtual memory, crashing the machines of the unwary ones who tried
> to build emacs without a ulimit...

This minimal patch "fixes" the issue here, by throwing a fatal error
in this unusual and avoidable situation. No new test as not testable.


Pip

[-- Attachment #2: 0001-Don-t-exhaust-memory-on-large-pure-space-allocations.patch --]
[-- Type: application/x-patch, Size: 982 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#46916: 28.0.50; pure_alloc(10424, ...) fails badly when pure space is exhausted
  2021-03-04 12:44 ` Pip Cet
@ 2021-03-04 14:15   ` Eli Zaretskii
  2021-10-31  2:56     ` Stefan Kangas
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2021-03-04 14:15 UTC (permalink / raw)
  To: Pip Cet; +Cc: 46916

> From: Pip Cet <pipcet@gmail.com>
> Date: Thu, 4 Mar 2021 12:44:49 +0000
> 
> On Thu, Mar 4, 2021 at 12:26 PM Pip Cet <pipcet@gmail.com> wrote:
> > Pure space should be removed. But until it is, it shouldn't exhaust
> > all virtual memory, crashing the machines of the unwary ones who tried
> > to build emacs without a ulimit...
> 
> This minimal patch "fixes" the issue here, by throwing a fatal error
> in this unusual and avoidable situation. No new test as not testable.

Just enlarge the pure space amount, and be done with it.  That's what
we always do when this happens.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#46916: 28.0.50; pure_alloc(10424, ...) fails badly when pure space is exhausted
  2021-03-04 14:15   ` Eli Zaretskii
@ 2021-10-31  2:56     ` Stefan Kangas
  2021-10-31  7:13       ` Eli Zaretskii
  2021-10-31  9:41       ` Pip Cet
  0 siblings, 2 replies; 10+ messages in thread
From: Stefan Kangas @ 2021-10-31  2:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 46916, Pip Cet

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Pip Cet <pipcet@gmail.com>
>> Date: Thu, 4 Mar 2021 12:44:49 +0000
>>
>> On Thu, Mar 4, 2021 at 12:26 PM Pip Cet <pipcet@gmail.com> wrote:
>> > Pure space should be removed. But until it is, it shouldn't exhaust
>> > all virtual memory, crashing the machines of the unwary ones who tried
>> > to build emacs without a ulimit...
>>
>> This minimal patch "fixes" the issue here, by throwing a fatal error
>> in this unusual and avoidable situation. No new test as not testable.
>
> Just enlarge the pure space amount, and be done with it.  That's what
> we always do when this happens.

There's a patch here that was never installed.  Eli didn't sound to
enthusiastic.  Is there anything more left to do here, or should this
just be closed?  Or should purespace be enlarged?





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#46916: 28.0.50; pure_alloc(10424, ...) fails badly when pure space is exhausted
  2021-10-31  2:56     ` Stefan Kangas
@ 2021-10-31  7:13       ` Eli Zaretskii
  2021-10-31  9:41       ` Pip Cet
  1 sibling, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2021-10-31  7:13 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 46916, pipcet

> From: Stefan Kangas <stefan@marxist.se>
> Date: Sat, 30 Oct 2021 19:56:42 -0700
> Cc: Pip Cet <pipcet@gmail.com>, 46916@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Pip Cet <pipcet@gmail.com>
> >> Date: Thu, 4 Mar 2021 12:44:49 +0000
> >>
> >> On Thu, Mar 4, 2021 at 12:26 PM Pip Cet <pipcet@gmail.com> wrote:
> >> > Pure space should be removed. But until it is, it shouldn't exhaust
> >> > all virtual memory, crashing the machines of the unwary ones who tried
> >> > to build emacs without a ulimit...
> >>
> >> This minimal patch "fixes" the issue here, by throwing a fatal error
> >> in this unusual and avoidable situation. No new test as not testable.
> >
> > Just enlarge the pure space amount, and be done with it.  That's what
> > we always do when this happens.
> 
> There's a patch here that was never installed.  Eli didn't sound to
> enthusiastic.  Is there anything more left to do here, or should this
> just be closed?  Or should purespace be enlarged?

We could enlarge the pure space size, yes.  but since no one has yet
reported any problems with that, I tend to do nothing for now.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#46916: 28.0.50; pure_alloc(10424, ...) fails badly when pure space is exhausted
  2021-10-31  2:56     ` Stefan Kangas
  2021-10-31  7:13       ` Eli Zaretskii
@ 2021-10-31  9:41       ` Pip Cet
  2021-10-31 11:29         ` Eli Zaretskii
  2021-10-31 14:02         ` Stefan Kangas
  1 sibling, 2 replies; 10+ messages in thread
From: Pip Cet @ 2021-10-31  9:41 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 46916

On Sun, Oct 31, 2021 at 2:56 AM Stefan Kangas <stefan@marxist.se> wrote:
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> >> From: Pip Cet <pipcet@gmail.com>
> >> Date: Thu, 4 Mar 2021 12:44:49 +0000
> >>
> >> On Thu, Mar 4, 2021 at 12:26 PM Pip Cet <pipcet@gmail.com> wrote:
> >> > Pure space should be removed. But until it is, it shouldn't exhaust
> >> > all virtual memory, crashing the machines of the unwary ones who tried
> >> > to build emacs without a ulimit...
> >>
> >> This minimal patch "fixes" the issue here, by throwing a fatal error
> >> in this unusual and avoidable situation. No new test as not testable.
> >
> > Just enlarge the pure space amount, and be done with it.  That's what
> > we always do when this happens.
>
> There's a patch here that was never installed.  Eli didn't sound to
> enthusiastic.  Is there anything more left to do here, or should this
> just be closed?  Or should purespace be enlarged?

I think Eli's suggestion was sarcastic, but I'm not sure.

I have no objections to this report being closed, in case that's
relevant: it's a clear bug but Eli appears to prefer not to fix it,
and I can live with that. In fact, isn't that what WONTFIX is for?

Thanks for taking the time to look into these issues!

Pip





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#46916: 28.0.50; pure_alloc(10424, ...) fails badly when pure space is exhausted
  2021-10-31  9:41       ` Pip Cet
@ 2021-10-31 11:29         ` Eli Zaretskii
  2021-10-31 14:02         ` Stefan Kangas
  1 sibling, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2021-10-31 11:29 UTC (permalink / raw)
  To: Pip Cet; +Cc: stefan, 46916

> From: Pip Cet <pipcet@gmail.com>
> Date: Sun, 31 Oct 2021 09:41:58 +0000
> Cc: Eli Zaretskii <eliz@gnu.org>, 46916@debbugs.gnu.org
> 
> On Sun, Oct 31, 2021 at 2:56 AM Stefan Kangas <stefan@marxist.se> wrote:
> >
> > Eli Zaretskii <eliz@gnu.org> writes:
> >
> > >> From: Pip Cet <pipcet@gmail.com>
> > >> Date: Thu, 4 Mar 2021 12:44:49 +0000
> > >>
> > >> On Thu, Mar 4, 2021 at 12:26 PM Pip Cet <pipcet@gmail.com> wrote:
> > >> > Pure space should be removed. But until it is, it shouldn't exhaust
> > >> > all virtual memory, crashing the machines of the unwary ones who tried
> > >> > to build emacs without a ulimit...
> > >>
> > >> This minimal patch "fixes" the issue here, by throwing a fatal error
> > >> in this unusual and avoidable situation. No new test as not testable.
> > >
> > > Just enlarge the pure space amount, and be done with it.  That's what
> > > we always do when this happens.
> >
> > There's a patch here that was never installed.  Eli didn't sound to
> > enthusiastic.  Is there anything more left to do here, or should this
> > just be closed?  Or should purespace be enlarged?
> 
> I think Eli's suggestion was sarcastic, but I'm not sure.

It wasn't, FWIW.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#46916: 28.0.50; pure_alloc(10424, ...) fails badly when pure space is exhausted
  2021-10-31  9:41       ` Pip Cet
  2021-10-31 11:29         ` Eli Zaretskii
@ 2021-10-31 14:02         ` Stefan Kangas
  2021-10-31 14:23           ` Eli Zaretskii
  1 sibling, 1 reply; 10+ messages in thread
From: Stefan Kangas @ 2021-10-31 14:02 UTC (permalink / raw)
  To: Pip Cet; +Cc: 46916

Pip Cet <pipcet@gmail.com> writes:

> I have no objections to this report being closed, in case that's
> relevant: it's a clear bug but Eli appears to prefer not to fix it,
> and I can live with that. In fact, isn't that what WONTFIX is for?

Hmm, so I reviewed all this more carefully, and I think I agree that we
might as well fix this bug.

OT1H, the use-case you present is quite unusual, as almost no one will
call `purespace' outside of Emacs core development.

OTOH, it is a pretty bad behavior you observed, and the patch seems
minimal enough to me.  IIUC, outside of bootstrapping Emacs, we
won't/shouldn't call 'purespace' very often, so the code-path will not
be exercised in normal use (which means your patch won't hurt), and it
might save someones system from swapping to death.

Eli, I'm not sure if your previous reply was just intended to say "we
normally just increase the size of purespace", or if you also object to
installing this patch?





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#46916: 28.0.50; pure_alloc(10424, ...) fails badly when pure space is exhausted
  2021-10-31 14:02         ` Stefan Kangas
@ 2021-10-31 14:23           ` Eli Zaretskii
  2021-11-05  7:11             ` Stefan Kangas
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2021-10-31 14:23 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 46916, pipcet

> From: Stefan Kangas <stefan@marxist.se>
> Date: Sun, 31 Oct 2021 07:02:47 -0700
> Cc: Eli Zaretskii <eliz@gnu.org>, 46916@debbugs.gnu.org
> 
> Eli, I'm not sure if your previous reply was just intended to say "we
> normally just increase the size of purespace", or if you also object to
> installing this patch?

I don't see a need for anything more complex than enlarging the pure
space, yes.  in fact, I don't even see a reason to enlarge the pure
space, since no one else reported a problem with the existing amount
of pure space.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#46916: 28.0.50; pure_alloc(10424, ...) fails badly when pure space is exhausted
  2021-10-31 14:23           ` Eli Zaretskii
@ 2021-11-05  7:11             ` Stefan Kangas
  0 siblings, 0 replies; 10+ messages in thread
From: Stefan Kangas @ 2021-11-05  7:11 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 46916, pipcet

tags 46916 wontfix
close 46916
thanks

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stefan Kangas <stefan@marxist.se>
>> Date: Sun, 31 Oct 2021 07:02:47 -0700
>> Cc: Eli Zaretskii <eliz@gnu.org>, 46916@debbugs.gnu.org
>>
>> Eli, I'm not sure if your previous reply was just intended to say "we
>> normally just increase the size of purespace", or if you also object to
>> installing this patch?
>
> I don't see a need for anything more complex than enlarging the pure
> space, yes.  in fact, I don't even see a reason to enlarge the pure
> space, since no one else reported a problem with the existing amount
> of pure space.

OK, so I'm closing this bug as wontfix.  I somewhat agree with Pip Cet
that we could just fix this, but given that the plan is to eventually
remove purespace, we also might as well leave it all alone.





^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-11-05  7:11 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-04 12:24 bug#46916: 28.0.50; pure_alloc(10424, ...) fails badly when pure space is exhausted Pip Cet
2021-03-04 12:44 ` Pip Cet
2021-03-04 14:15   ` Eli Zaretskii
2021-10-31  2:56     ` Stefan Kangas
2021-10-31  7:13       ` Eli Zaretskii
2021-10-31  9:41       ` Pip Cet
2021-10-31 11:29         ` Eli Zaretskii
2021-10-31 14:02         ` Stefan Kangas
2021-10-31 14:23           ` Eli Zaretskii
2021-11-05  7:11             ` Stefan Kangas

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).