unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* sqlite memory allocation and async signal safety
       [not found] <8735j7btyj.fsf.ref@yahoo.com>
@ 2022-03-24  9:06 ` Po Lu
  2022-03-24  9:53   ` Eli Zaretskii
  0 siblings, 1 reply; 9+ messages in thread
From: Po Lu @ 2022-03-24  9:06 UTC (permalink / raw)
  To: emacs-devel

SQLite allocates various pieces of memory inside most of its functions.
This is done through the C library malloc, which immediately raises the
reentrancy question: shouldn't input be blocked around such functions?

Secondly, SQLite returns an error code upon running out of memory in an
irrecoverable fashion.  Shouldn't the code that currently signals an
ordinary error in such situations call `memory_full' instead?

Alternatively, we could make SQLite call xmalloc instead of C library
malloc, but I suppose the SQLite library is better at coping with
allocation failures of large sizes than xmalloc is, so that's probably
not the best solution.

Ideas?



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

* Re: sqlite memory allocation and async signal safety
  2022-03-24  9:06 ` sqlite memory allocation and async signal safety Po Lu
@ 2022-03-24  9:53   ` Eli Zaretskii
  2022-03-24 10:21     ` Po Lu
                       ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Eli Zaretskii @ 2022-03-24  9:53 UTC (permalink / raw)
  To: Po Lu; +Cc: emacs-devel

> From: Po Lu <luangruo@yahoo.com>
> Date: Thu, 24 Mar 2022 17:06:28 +0800
> 
> SQLite allocates various pieces of memory inside most of its functions.
> This is done through the C library malloc, which immediately raises the
> reentrancy question: shouldn't input be blocked around such functions?

I think we used to block input around function calls that could
allocate memory because our signal handlers, and in particular SIGIO
handler, did non-trivial stuff.  Nowadays our signal handlers just set
a flag and return, so I'm not sure this is needed anymore.  Especially
when system library malloc is called, which AFAIU is mostly async-safe
nowadays on modern platforms.

Am I missing something?

> Secondly, SQLite returns an error code upon running out of memory in an
> irrecoverable fashion.  Shouldn't the code that currently signals an
> ordinary error in such situations call `memory_full' instead?

memory_full means basically that the user should save and exit ASAP.
Is that indeed required when SQLite runs out of memory?  Isn't it
enough to just fail the SQLite-related function?

> Alternatively, we could make SQLite call xmalloc instead of C library
> malloc, but I suppose the SQLite library is better at coping with
> allocation failures of large sizes than xmalloc is, so that's probably
> not the best solution.

I see no reason to go to our malloc as long as SQLite is not supposed
to be invoked before dumping Emacs.

I'd like to hear others about these issues, though, as I'm not enough
of an expert on this stuff.



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

* Re: sqlite memory allocation and async signal safety
  2022-03-24  9:53   ` Eli Zaretskii
@ 2022-03-24 10:21     ` Po Lu
  2022-03-24 11:08       ` Eli Zaretskii
  2022-03-24 17:13       ` Stefan Monnier
  2022-03-24 10:21     ` Andreas Schwab
  2022-03-24 10:22     ` Po Lu
  2 siblings, 2 replies; 9+ messages in thread
From: Po Lu @ 2022-03-24 10:21 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> I think we used to block input around function calls that could
> allocate memory because our signal handlers, and in particular SIGIO
> handler, did non-trivial stuff.  Nowadays our signal handlers just set
> a flag and return, so I'm not sure this is needed anymore.  Especially
> when system library malloc is called, which AFAIU is mostly async-safe
> nowadays on modern platforms.
>
> Am I missing something?

Unfortunately, most system malloc implementations are still not
async-signal safe, but if all that happens is a flag being set, then I
don't think calling block_input is required anymore.

Which flag is that, and where is it tested?

> memory_full means basically that the user should save and exit ASAP.
> Is that indeed required when SQLite runs out of memory?  Isn't it
> enough to just fail the SQLite-related function?

That's why I asked the second question: SQLite is supposedly very good
at coping with allocation failures, so when the memory allocation fails,
it probably means there is really no memory left.

> I see no reason to go to our malloc as long as SQLite is not supposed
> to be invoked before dumping Emacs.
>
> I'd like to hear others about these issues, though, as I'm not enough
> of an expert on this stuff.

Indeed, thanks for clarifying.



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

* Re: sqlite memory allocation and async signal safety
  2022-03-24  9:53   ` Eli Zaretskii
  2022-03-24 10:21     ` Po Lu
@ 2022-03-24 10:21     ` Andreas Schwab
  2022-03-24 11:03       ` Eli Zaretskii
  2022-03-24 10:22     ` Po Lu
  2 siblings, 1 reply; 9+ messages in thread
From: Andreas Schwab @ 2022-03-24 10:21 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Po Lu, emacs-devel

On Mär 24 2022, Eli Zaretskii wrote:

> when system library malloc is called, which AFAIU is mostly async-safe
> nowadays on modern platforms.

malloc is definitely not async-safe.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."



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

* Re: sqlite memory allocation and async signal safety
  2022-03-24  9:53   ` Eli Zaretskii
  2022-03-24 10:21     ` Po Lu
  2022-03-24 10:21     ` Andreas Schwab
@ 2022-03-24 10:22     ` Po Lu
  2 siblings, 0 replies; 9+ messages in thread
From: Po Lu @ 2022-03-24 10:22 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> I think we used to block input around function calls that could
> allocate memory because our signal handlers, and in particular SIGIO
> handler, did non-trivial stuff.  Nowadays our signal handlers just set
> a flag and return, so I'm not sure this is needed anymore.  Especially
> when system library malloc is called, which AFAIU is mostly async-safe
> nowadays on modern platforms.
>
> Am I missing something?

Unfortunately, most system malloc implementations are still not
async-signal safe, but if all that happens is a flag being set, then I
don't think calling block_input is required anymore.

Which flag is that, and where is it tested?

> memory_full means basically that the user should save and exit ASAP.
> Is that indeed required when SQLite runs out of memory?  Isn't it
> enough to just fail the SQLite-related function?

That's why I asked the second question: SQLite is supposedly very good
at coping with allocation failures, so when the memory allocation fails,
it probably means there is really no memory left.

> I see no reason to go to our malloc as long as SQLite is not supposed
> to be invoked before dumping Emacs.
>
> I'd like to hear others about these issues, though, as I'm not enough
> of an expert on this stuff.

Indeed, thanks for clarifying.



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

* Re: sqlite memory allocation and async signal safety
  2022-03-24 10:21     ` Andreas Schwab
@ 2022-03-24 11:03       ` Eli Zaretskii
  0 siblings, 0 replies; 9+ messages in thread
From: Eli Zaretskii @ 2022-03-24 11:03 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: luangruo, emacs-devel

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: Po Lu <luangruo@yahoo.com>,  emacs-devel@gnu.org
> Date: Thu, 24 Mar 2022 11:21:41 +0100
> 
> On Mär 24 2022, Eli Zaretskii wrote:
> 
> > when system library malloc is called, which AFAIU is mostly async-safe
> > nowadays on modern platforms.
> 
> malloc is definitely not async-safe.

OK, but still: we no longer call malloc in our signal handlers, I
think.



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

* Re: sqlite memory allocation and async signal safety
  2022-03-24 10:21     ` Po Lu
@ 2022-03-24 11:08       ` Eli Zaretskii
  2022-03-24 11:51         ` Po Lu
  2022-03-24 17:13       ` Stefan Monnier
  1 sibling, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2022-03-24 11:08 UTC (permalink / raw)
  To: Po Lu; +Cc: emacs-devel

> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Thu, 24 Mar 2022 18:21:29 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I think we used to block input around function calls that could
> > allocate memory because our signal handlers, and in particular SIGIO
> > handler, did non-trivial stuff.  Nowadays our signal handlers just set
> > a flag and return, so I'm not sure this is needed anymore.  Especially
> > when system library malloc is called, which AFAIU is mostly async-safe
> > nowadays on modern platforms.
> >
> > Am I missing something?
> 
> Unfortunately, most system malloc implementations are still not
> async-signal safe, but if all that happens is a flag being set, then I
> don't think calling block_input is required anymore.
> 
> Which flag is that, and where is it tested?

It depends on the signal.  Look at the signal handlers we install.
For SIGIO, this is the handler:

  void
  handle_input_available_signal (int sig)
  {
    pending_signals = true;

    if (input_available_clear_time)
      *input_available_clear_time = make_timespec (0, 0);
  }

and the flag is pending_signals.



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

* Re: sqlite memory allocation and async signal safety
  2022-03-24 11:08       ` Eli Zaretskii
@ 2022-03-24 11:51         ` Po Lu
  0 siblings, 0 replies; 9+ messages in thread
From: Po Lu @ 2022-03-24 11:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> It depends on the signal.  Look at the signal handlers we install.
> For SIGIO, this is the handler:
>
>   void
>   handle_input_available_signal (int sig)
>   {
>     pending_signals = true;
>
>     if (input_available_clear_time)
>       *input_available_clear_time = make_timespec (0, 0);
>   }
>
> and the flag is pending_signals.

Thanks.  From what I can tell, it's checked in maybe_quit (and some
other places such as unblock_input), and it shouldn't require input to
be blocked around calls to sqlite functions.

The other signal handlers seem fine too.



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

* Re: sqlite memory allocation and async signal safety
  2022-03-24 10:21     ` Po Lu
  2022-03-24 11:08       ` Eli Zaretskii
@ 2022-03-24 17:13       ` Stefan Monnier
  1 sibling, 0 replies; 9+ messages in thread
From: Stefan Monnier @ 2022-03-24 17:13 UTC (permalink / raw)
  To: Po Lu; +Cc: Eli Zaretskii, emacs-devel

> Unfortunately, most system malloc implementations are still not
> async-signal safe,

Since we don't do any malloc inside our signal handler, this is not
an issue.  What Eli meant I think by "async safe" is in terms of
multithreading with pthreads.


        Stefan




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

end of thread, other threads:[~2022-03-24 17:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <8735j7btyj.fsf.ref@yahoo.com>
2022-03-24  9:06 ` sqlite memory allocation and async signal safety Po Lu
2022-03-24  9:53   ` Eli Zaretskii
2022-03-24 10:21     ` Po Lu
2022-03-24 11:08       ` Eli Zaretskii
2022-03-24 11:51         ` Po Lu
2022-03-24 17:13       ` Stefan Monnier
2022-03-24 10:21     ` Andreas Schwab
2022-03-24 11:03       ` Eli Zaretskii
2022-03-24 10:22     ` Po Lu

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).