This is still a problem.

Using a user buffer will require gc-protecting it and thus a major overhaul, so I think it's not a good idea.

IMO what we should do is: if we fail to allocate, we discard the original signal and replace it with an OOM signal (pointing to constants so requiring no allocation). Perhaps we should make a new field in emacs_funcall_exit for OOM, or we can just use emacs_funcall_exit_signal.

Alternatively, make a copy_emacs_value function that allows the user to copy the signal out, returning NULL to let the caller know that an allocation failure occurred.



On Thu, Sep 7, 2023 at 5:24 AM Philipp Stephani <phst@google.com> wrote:
On Thu, 7 Sept 2023 at 09:07, Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Xinyang Chen <chenxinyang99@gmail.com>
> > Date: Wed, 6 Sep 2023 18:52:14 -0400
> >
> > Currently `module_non_local_exit_get` returns pointers to fields
> > in emacs_env_private:
> > ```
> >   if (p->pending_non_local_exit != emacs_funcall_exit_return)
> >     {
> >       *symbol = &p->non_local_exit_symbol;
> >       *data = &p->non_local_exit_data;
> >     }
> > ```
> > this means that if one tries to:
> > ```
> > funcall(...);
> > non_local_exit_get(&s1, &d1);
> > funcall(...);
> > non_local_exit_get(&s2, &d2);
> > non_local_exit_signal(s1, d1);
> > ```
> > you would signal the second error, instead of the first error (I expected
> > this to happen).
> > It seems to me that `module_non_local_exit_get` should
> > `allocate_emacs_value` instead.
>
> Philipp, Daniel: any comments?

Nice find!
We can't use allocate_emacs_value here because non_local_exit_get has
to work in OOM situations and can never fail. What we could do here is
e.g.:
- Document the current behavior, stating that the emacs_value objects
returned from non_local_exit_get are ephemeral. I'm not a huge fan of
this because it makes non_local_exit_get behave different from all
other functions.
- Provide an alternative non_local_exit_copy that copies the 2
Lisp_Objects into an opaque buffer supplied by the user (plus a way to
determine the buffer size). That way we shift the responsibility of
dealing with allocation failures to the user.
- Attempt to allocate a new emacs_value, fall back to the current
behavior if that fails. I don't really like that option either because
it doesn't solve the initial problem in all cases (so users still need
to deal with it), but makes both the interface and the implementation
more complex.
- Crash if we can't allocate memory. That has been rejected in other cases.

>
> Btw, the non_local_exit_get function is currently not documented in
> the ELisp manual; should it be?

At least in Emacs 29 I see it documented ("Module Nonlocal" node).


--
Google Germany GmbH
Erika-Mann-Straße 33
80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Falls Sie diese fälschlicherweise
erhalten haben sollten, leiten Sie diese bitte nicht an jemand anderes
weiter, löschen Sie alle Kopien und Anhänge davon und lassen Sie mich
bitte wissen, dass die E-Mail an die falsche Person gesendet wurde.

This e-mail is confidential. If you received this communication by
mistake, please don’t forward it to anyone else, please erase all
copies and attachments, and please let me know that it has gone to the
wrong person.