Re: Question about Ctrl-C and less

Martijn van Oosterhout wrote:
> On Tue, Oct 18, 2005 at 09:32:25PM -0700, Kevin Brown wrote:
> > So I think the right answer here is for psql to handle SIGINT
> > internally by doing a pclose() first (and at this point, it probably
> > should ignore SIGINT altogether), then returning to the main loop
> > (and, of course, cleaning up anything that needs it along the way).
> > If the child hasn't exited then pclose() will block until it has. The
> > end result should be the semantics you want: if psql is in the middle
> > of sending a bunch of rows of output to the pager, this will interrupt
> > that process. If the pager remains running then it will hopefully
> > give the user the ability to scroll through whatever results were sent
> > to it.
>
> That's what I meant by "more comprehensive patch". Basically, the
> longjmp has to go because it leaks memory (and file descriptors) and
> doesn't allow you to control things at the right level. My little patch
> basically set the signal handler *after* the popen so everything works
> as expected.

It does? It looked to me like it was setting the signal handler to
SIG_IGN before doing the popen(), and resetting it to the internal
signal handler after doing the pclose().

Am I examining the wrong patch?

> My plan is to have the interrupt handler set a flag "control_c_pressed"
> and check it at strategic points. Then memory can be deallocated and
> returned properly. It's a lot more invasive and the corners cases will
> be "interesting".

If I were to take that approach, I'd do the check immediately after
writing to the pipe for sure, and possibly after reading from the
database handle. I'd have to look at the code to see what's going on
and thus where the best places to stick the checks are. But in
general, you want to put a check anywhere within the loop that is
immediately after a blocking operation such as a read or write.
However, that said...

The semantics for handling SIGINT here should be pretty much the same
as they are when there's no pager at all (which I know works because I
can interrupt the output anytime and get the psql prompt back). In
fact, this suggests that the proper course of action would be to store
the pipe's file descriptor someplace accessible to the normal SIGINT
handler, and have the SIGINT handler check its value upon receipt of
SIGINT. If the value is nonzero then pclose() it and then proceed as
before (and zero the descriptor after any pclose() so that the handler
always does the right thing). If there's any additional memory that
has to be freed, then do that too, of course.

That'll take care of the descriptor leak and it'll get you the right
semantics as a nice bonus.

> Your point about SIGQUIT is valid. I didn't include it in my patch
> since I wasn't sure what the expected behaviour would be. If I got core
> files everytime I pressed SIGQUIT, I'd have a lot of core files
> scattered around my disk; one of the reasons I disable core files by
> default.

If you're hitting SIGQUIT a lot then perhaps you need to assign it to
a different key. :-)

Seriously, you shouldn't be using SIGQUIT unless you want a core file.
Otherwise, use SIGINT. If the application is unresponsive to SIGINT,
my normal course of action is to suspend it via job control signals
(i.e., send SIGTSTP) and then send it a SIGTERM via the kill command.
If that doesn't do it, then it gets a SIGKILL. And if it doesn't stop
upon receipt of SIGTSTP I kill it externally and complain to its
developers (assuming it isn't hung in some uninterruptible sleep in
the kernel or something).

> OTOH, if I wanted to trap psql, I'd run it under a debugger or attach
> one which would catch SIGQUIT even if the program ignores it.

Suppose it's hung on a really hard to reproduce condition? Point
being that you don't necessarily know ahead of time that you're going
to want a core file. Otherwise SIGQUIT wouldn't have the default
semantics it has, and it wouldn't have a terminal key associated with
it. The guys that designed this stuff knew what they were doing.

--
Kevin Brown kevin(at)sysexperts(dot)com