Обсуждение: Question about Ctrl-C and less

This behaviour has been around so long that I've gotten used to it but
I've always considered it a bug. Yet it has never been fixed so I'm
going to ask if anybody else has issues with this behaviour.

Reproducing it is easy:

1. Set PAGER=less
2. Start psql
3. Type: \df
4. When output appears, hit Control-C

Now watch as both psql and less try to read your characters, each
getting about half and getting very confused. Even if you manage to get
one of the two to quit, your terminal is still left in a scrambled
state, generally requiring a soft-reset on the terminal.

The fix is easy: where currently the code with popen/pclose ignores
SIGPIPE, tell it to also ignore SIGINT and restore the normal signal
handler on quit. This is almost in line with the system() function
which blocks SIGQUIT and SIGINT while the subprocess is running.

This problem has been around for ever yet obviously not everybody runs
into it all the time like I do. Would patch to fix this be accepted or
is there a reason why not?

Actually, I'm somewhat in favour if getting rid of the longjmp from the
signal handler and instead setting a flag to be checked at strategic
points in the code to abort whatever it is doing. The current way leaks
memory like crazy. If you're worried about infinite loops there's
always SIGQUIT.

Thanks in advance,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

Martijn van Oosterhout <kleptog@svana.org> writes:
> This problem has been around for ever yet obviously not everybody runs
> into it all the time like I do. Would patch to fix this be accepted or
> is there a reason why not?

I guess everybody else uses "q" not control-C to get out of less ;-)

I'm not sure I agree with the concept of blocking SIGINT in this context
--- you would not, I trust, propose blocking SIGQUIT or SIGHUP, but then
why is it ok to block SIGINT?

AFAICT it wouldn't fix the problem anyway, as there's still a bug in
less: it doesn't restore the terminal settings on exit.
        regards, tom lane

On Sun, Oct 16, 2005 at 02:44:41PM -0400, Tom Lane wrote:
> Martijn van Oosterhout <kleptog@svana.org> writes:
> > This problem has been around for ever yet obviously not everybody runs
> > into it all the time like I do. Would patch to fix this be accepted or
> > is there a reason why not?
>
> I guess everybody else uses "q" not control-C to get out of less ;-)

Umm, Ctrl-C doesn't quit less, it aborts the current command. So if
it's doing a search or you've hit "F" (follow) or any number of other
things then the only way out is to press Ctrl-C. You have to press "q"
to quit less like you say.

> I'm not sure I agree with the concept of blocking SIGINT in this context
> --- you would not, I trust, propose blocking SIGQUIT or SIGHUP, but then
> why is it ok to block SIGINT?

The system() function blocks -INT and -QUIT, as does the shell when you
run a program, so why not?

> AFAICT it wouldn't fix the problem anyway, as there's still a bug in
> less: it doesn't restore the terminal settings on exit.

The point is, less is still running but psql it pulling the keystrokes
from under it... When you finally quit less it does restore the
settings, but now readline doesn't expect that as it's changed them
again...

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

Martijn van Oosterhout wrote:

>The point is, less is still running but psql it pulling the keystrokes
>from under it... When you finally quit less it does restore the
>settings, but now readline doesn't expect that as it's changed them
>again...
>
>  
>

Martin,

Let's see the patch. I assume it should be fairly small. If we could get 
it in early in the 8.2 cycle we would have plenty of time to bang on it. 
In principle this sounds reasonable to me, but psql can be broken quite 
easily - I know as I've done it a couple of times ;-)

cheers

andrew

Martjin,

> This problem has been around for ever yet obviously not everybody runs
> into it all the time like I do. Would patch to fix this be accepted or
> is there a reason why not?

Actually, I run into it fairly often when I'm being hasty.  I'd imagine most 
Linux-based newbies do as well.

-- 
Josh Berkus
Aglio Database Solutions
San Francisco

On Sun, Oct 16, 2005 at 04:06:04PM -0400, Andrew Dunstan wrote:
> Martin,
>
> Let's see the patch. I assume it should be fairly small. If we could get
> it in early in the 8.2 cycle we would have plenty of time to bang on it.
> In principle this sounds reasonable to me, but psql can be broken quite
> easily - I know as I've done it a couple of times ;-)

Very well, patch attached. It's quite simple actually. However, there
seems to be some push back from people suggesting that jumping back to
the main loop before the pager has quit is not buggy behaviour.
Assuming that a ^C will kill the pager is just folly.

Tom asked if we should be blocking SIGQUIT and SIGHUP too. Standard
procedure for spawning external interactive processes includes blocking
SIGQUIT too (see system() manpage). Logically speaking, when the user
sends an interrupt from the keyboard they expect to interrupt the
currently active *interaxtive* process. Hence, once psql has spawned
the pager, it should ignore such interrupts until control is returned
(after pclose). So yes, I would suggest blocking SIGQUIT also, if only
to prevent terminal corruption problems. Interactive programs like less
trap SIGQUIT to restore the terminal settings on exit, but the exit
anyway.

SIGHUP is different. It is sent by the system, not the user, indicating
the terminal has disconnected. psql is not a daemon expecting to
survive that and hence should quit as normal, along with all other
processes on that terminal.

If this isn't going to make 8.1 anyway I'd probably go for a patch that
got rid of the longjmp altogether (if people will accept that), fixing
the memory leaks at the same time. At the moment I'm just looking for a
concensus that it's a problem to be solved.

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

On Sun, Oct 16, 2005 at 01:57:13PM -0700, Josh Berkus wrote:
> Martjin,
> > This problem has been around for ever yet obviously not everybody runs
> > into it all the time like I do. Would patch to fix this be accepted or
> > is there a reason why not?
> Actually, I run into it fairly often when I'm being hasty.  I'd imagine most 
> Linux-based newbies do as well.

I believe many such applications (including less?) are following the
Emacs tradition(?) of having Control-C, or often, Control-G (with
Control-G actually being re-bound as the interrupt character) - mean
interrupt the current operation within the application - not cause
the application to exit. If I wanted to quit, I'd hit 'q'. The times
I do hit Control-C, or Control-G, are when the application I believe
I am using (my keystrokes are interacting with) is performing a
lengthy operation, that I wish to interrupt.

One such operation that has come up with me in less, is when it is
attempting to calculate the line numbers. It becomes non-responsive
during this time, and if I don't care what the line number is, I
interrupt less to tell it 'no - don't calculate line numbers - get
ready for my next command'.

So, I'm with Martijn. It's not right for psql to die from SIGINT
during the execution of another command. SIGINT is a user signal
that can be generated from a keyboard. Psql should not be making
assumptions about what the signal is meant to mean, to the current
application in control of the keyboard.

I agree with Martijn on SIGQUIT (another keyboard signal!) being
ignored, but leaving SIGHUP (not a keyboard signal!) cause psql to
exit. SIGHUP is usually sent to everybody, with everybody paying
attention to it. SIGINT and SIGQUIT are intended for the application
in control of the terminal.

Cheers,
mark

-- 
mark@mielke.cc / markm@ncf.ca / markm@nortel.com     __________________________
.  .  _  ._  . .   .__    .  . ._. .__ .   . . .__  | Neighbourhood Coder
|\/| |_| |_| |/    |_     |\/|  |  |_  |   |/  |_   | 
|  | | | | \ | \   |__ .  |  | .|. |__ |__ | \ |__  | Ottawa, Ontario, Canada
 One ring to rule them all, one ring to find them, one ring to bring them all                      and in the darkness
bindthem...
 
                          http://mark.mielke.cc/

I run into this problem sometimes, especially when I realize that
the query I've just started is going to run for a very long time and
not really provide anything useful.  I find that I have to close the
shell window to get out of it, and I'm always a bit uncomforatble
doing that.

-Kevin


>>> Martijn van Oosterhout <kleptog@svana.org>  >>>
At the moment I'm just looking for a
concensus that it's a problem to be solved.

On Tue, Oct 18, 2005 at 05:15:20PM -0500, Kevin Grittner wrote:
> I run into this problem sometimes, especially when I realize that
> the query I've just started is going to run for a very long time and
> not really provide anything useful.  I find that I have to close the
> shell window to get out of it, and I'm always a bit uncomforatble
> doing that.

Hmm, I'm glad it isn't just me experiencing this on a regular basis.
It's possibly also because Debian sets the default pager to less, which
makes it happen on every machine I use.

The simple patch I posted seems unlikely to make it into 8.1 so I'll
add a more comprehensive patch to my list hopefully for 8.2...

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

Martijn van Oosterhout wrote:
> Very well, patch attached. It's quite simple actually. However, there
> seems to be some push back from people suggesting that jumping back to
> the main loop before the pager has quit is not buggy behaviour.
> Assuming that a ^C will kill the pager is just folly.

Making assumptions about what the pager will do upon receipt of SIGINT
is folly as well.

Setting up SIGINT to be ignored may be the right answer (I don't
believe it is -- see below), but if so then it needs to be done
properly.  If it gets ignored prior to the popen(), then the child
will also end up ignoring it by default, because signal disposition is
inherited by child processes.  If we ignore SIGINT, it should be after
the popen(), not before.


When the user sends SIGINT, he means to interrupt whatever processing
is currently occurring.  He expects to regain control of the
terminal.  If psql is in the process of sending data to the pager,
then a SIGINT should cause psql to stop doing so.

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.


> Tom asked if we should be blocking SIGQUIT and SIGHUP too. Standard
> procedure for spawning external interactive processes includes blocking
> SIGQUIT too (see system() manpage).

SIGQUIT has a different standard meaning in Unix than SIGINT: it
causes the process to drop core.  We should not be blocking it -- we
should be leaving it alone.  The reason is that it's quite possible
that the user wants to have psql generate a core file while it's
writing output to the pager.


> Logically speaking, when the user sends an interrupt from the
> keyboard they expect to interrupt the currently active *interaxtive*
> process.

They expect to interrupt the currently active processing.  Not quite
the same thing.


> Hence, once psql has spawned
> the pager, it should ignore such interrupts until control is returned
> (after pclose). So yes, I would suggest blocking SIGQUIT also, if only
> to prevent terminal corruption problems. Interactive programs like less
> trap SIGQUIT to restore the terminal settings on exit, but the exit
> anyway.

They should be dropping core upon receipt of SIGQUIT.  It might be
nice if they cleaned up the terminal first, but receipt of a SIGQUIT
generally means that the user wants to run the resulting core file
through a debugger, and trapping the signal could alter the stack such
that the resulting core would be less useful.  I'd rather have to
clean up the terminal manually than have an unusable core file.





-- 
Kevin Brown                          kevin@sysexperts.com

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.

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

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.

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.

Anyway, thanks for the response, hopefully we can get this sorted out
in a later release...
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

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@sysexperts.com

On 2005-10-19, Kevin Brown <kevin@sysexperts.com> wrote:
> Making assumptions about what the pager will do upon receipt of SIGINT
> is folly as well.
>
> Setting up SIGINT to be ignored may be the right answer (I don't
> believe it is -- see below), but if so then it needs to be done
> properly.  If it gets ignored prior to the popen(), then the child
> will also end up ignoring it by default, because signal disposition is
> inherited by child processes.  If we ignore SIGINT, it should be after
> the popen(), not before.

I do not believe it is possible to do the signal disposition correctly
and still use popen() to run the pager. (You would need to reimplement
popen using raw syscalls.)

> So I think the right answer here is for psql to handle SIGINT
> internally by doing a pclose() first

The chances that psql can do this safely approach zero. pclose() is not a
signal-safe function, so it can only be called from a signal handler if
you _know_ that the signal did not interrupt any non-signal-safe function.
(Nor can the signal handler longjmp out in such a case, unless the code is
never again going to call any unsafe function.)

-- 
Andrew, Supernews
http://www.supernews.com - individual and corporate NNTP services

On Wed, Oct 19, 2005 at 04:24:21AM -0700, Kevin Brown wrote:
> 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().

Oops, you're right. It works because less sets it's own signal handler,
as does more. Sure, the status is inherited but I doubt there's a pager
out there that doesn't set its own signal handlers. Still, best do it
properly.

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

Well, the pager is only run after all the data has been collected from
the server so none of this is an issue while the query is being
processed. Send the cancel request and toss the data. Once we start the
pager just check after each line is printed.

> 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

<snip>

You can't do a pclose in a signal handler, it's not one of the
"reeentrant safe" functions and could lead to deadlocks. The signal
manpage documents the ones you can use. Just set a flag. Setting the
descriptor to NULL is worse because then we have check before every
output function. fprintf(NULL, ...) will segfault on most
architechtures I wager.

Also, the signal handler can't free the memory, there's way too many
arrays. A flag allows the output functions to exit sanely.

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

I use SIGQUIT to get out of the situation you get currently when you
press Ctrl-C to interrupt less. Somehow all the "q"s go to psql and all
the Ctrl-Ds to less. SIGQUIT just kills them both from where I can
clean the terminal up. So I will press SIGQUIT less once we fix up psql
to not do stupid things on SIGINT. :)

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

Martijn van Oosterhout wrote:
> You can't do a pclose in a signal handler, it's not one of the
> "reeentrant safe" functions and could lead to deadlocks. The signal
> manpage documents the ones you can use. Just set a flag. Setting the
> descriptor to NULL is worse because then we have check before every
> output function. fprintf(NULL, ...) will segfault on most
> architechtures I wager. 

Yeah, I was thinking that you'd do the check for the flag and invoke a
cleanup handler after the write() to the output file descriptor.  It's
not clear that you'd need to do the check anyplace else.  It's been a
while since I've messed with this stuff, but if I recall correctly,
the write() will return immediately after receipt of a signal, and
will indicate how much was actually written.  So receipt of a SIGINT
should wind up being handled in a reasonably timely fashion.

Additionally the normal SIGINT signal handler (the one that gets
invoked when the pager is turned off) can be called from the cleanup
handler in order to maintain the proper semantics.


-- 
Kevin Brown                          kevin@sysexperts.com

On Thu, Oct 20, 2005 at 03:42:10PM -0700, Kevin Brown wrote:
> Martijn van Oosterhout wrote:
> > You can't do a pclose in a signal handler, it's not one of the
> > "reeentrant safe" functions and could lead to deadlocks. The signal
> > manpage documents the ones you can use. Just set a flag. Setting the
> > descriptor to NULL is worse because then we have check before every
> > output function. fprintf(NULL, ...) will segfault on most
> > architechtures I wager. 
> Yeah, I was thinking that you'd do the check for the flag and invoke a
> cleanup handler after the write() to the output file descriptor.  It's
> not clear that you'd need to do the check anyplace else.  It's been a
> while since I've messed with this stuff, but if I recall correctly,
> the write() will return immediately after receipt of a signal, and
> will indicate how much was actually written.  So receipt of a SIGINT
> should wind up being handled in a reasonably timely fashion.

> Additionally the normal SIGINT signal handler (the one that gets
> invoked when the pager is turned off) can be called from the cleanup
> handler in order to maintain the proper semantics.

I disagree that psql should make *any* assumptions about what SIGINT
means to the child process. Consider less again, and Control-C used
to abort a search. You are suggesting that Control-C should not only
abort the search, but should also cut off the input from less. Less
won't die. Less will just see a terminated input stream. What has been
gained from this? Is this intuitive behaviour?

If the pager does die in response to SIGINT, the write() will fail with
SIGPIPE. Completely clean, without any need for psql to pay attention
to SIGINT.

I think the only reasonable behaviour is to ignore SIGINT within the
parent, until the child exits. I don't see why other behaviours are
even being considered. To me, it points at a misunderstanding of the
problem.

Cheers,
mark

-- 
mark@mielke.cc / markm@ncf.ca / markm@nortel.com     __________________________
.  .  _  ._  . .   .__    .  . ._. .__ .   . . .__  | Neighbourhood Coder
|\/| |_| |_| |/    |_     |\/|  |  |_  |   |/  |_   | 
|  | | | | \ | \   |__ .  |  | .|. |__ |__ | \ |__  | Ottawa, Ontario, Canada
 One ring to rule them all, one ring to find them, one ring to bring them all                      and in the darkness
bindthem...
 
                          http://mark.mielke.cc/

On Thu, Oct 20, 2005 at 08:11:14PM -0400, mark@mark.mielke.cc wrote:
> I disagree that psql should make *any* assumptions about what SIGINT
> means to the child process. Consider less again, and Control-C used
> to abort a search. You are suggesting that Control-C should not only
> abort the search, but should also cut off the input from less. Less
> won't die. Less will just see a terminated input stream. What has been
> gained from this? Is this intuitive behaviour?

I must say I agree with the idea that Ctrl-C shouldn't stop the stream
from psql, but I'm willing to let it slide because a lot of other
programs work this way. I imagine asking it to be configurable will
meet even more resistance.

> I think the only reasonable behaviour is to ignore SIGINT within the
> parent, until the child exits. I don't see why other behaviours are
> even being considered. To me, it points at a misunderstanding of the
> problem.

I've been playing with a version of psql which on Ctrl-C doesn't
longjmp() but politely frees everything, waits for the pager and then
back to the main loop with the message "Interrupted". But now we have
another behaviour change: How to abort the gets() when you don't have
readline?

Doing it with a flag is a lot more susceptable to subtle behaviour
changes, but I'll see if I can make it work.

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

On Fri, Oct 21, 2005 at 01:53:32PM +0200, Martijn van Oosterhout wrote:
> On Thu, Oct 20, 2005 at 08:11:14PM -0400, mark@mark.mielke.cc wrote:
> > I disagree that psql should make *any* assumptions about what SIGINT
> > means to the child process. Consider less again, and Control-C used
> > to abort a search. You are suggesting that Control-C should not only
> > abort the search, but should also cut off the input from less. Less
> > won't die. Less will just see a terminated input stream. What has been
> > gained from this? Is this intuitive behaviour?
> I must say I agree with the idea that Ctrl-C shouldn't stop the stream
> from psql, but I'm willing to let it slide because a lot of other
> programs work this way. I imagine asking it to be configurable will
> meet even more resistance.

Which other ones? I can't think of one. The ones that don't handle
SIGINT, or that are not designed for this scenario certainly don't count -
as that is how psql works right now without the patch. Of the remaining
programs that are designed to work with an application such as less, which
ones purposefully close the input stream to less, and continue running?

I think 0.

> > I think the only reasonable behaviour is to ignore SIGINT within the
> > parent, until the child exits. I don't see why other behaviours are
> > even being considered. To me, it points at a misunderstanding of the
> > problem.
> I've been playing with a version of psql which on Ctrl-C doesn't
> longjmp() but politely frees everything, waits for the pager and then
> back to the main loop with the message "Interrupted". But now we have
> another behaviour change: How to abort the gets() when you don't have
> readline?
> Doing it with a flag is a lot more susceptable to subtle behaviour
> changes, but I'll see if I can make it work.

SIG_IGN is the easiest, and the most effective. Certainly the easiest
to maintain, and the least intrusive to the core.

I don't understand.

Cheers,
mark

-- 
mark@mielke.cc / markm@ncf.ca / markm@nortel.com     __________________________
.  .  _  ._  . .   .__    .  . ._. .__ .   . . .__  | Neighbourhood Coder
|\/| |_| |_| |/    |_     |\/|  |  |_  |   |/  |_   | 
|  | | | | \ | \   |__ .  |  | .|. |__ |__ | \ |__  | Ottawa, Ontario, Canada
 One ring to rule them all, one ring to find them, one ring to bring them all                      and in the darkness
bindthem...
 
                          http://mark.mielke.cc/

On Fri, Oct 21, 2005 at 08:48:31AM -0400, mark@mark.mielke.cc wrote:
> Which other ones? I can't think of one. The ones that don't handle
> SIGINT, or that are not designed for this scenario certainly don't count -
> as that is how psql works right now without the patch. Of the remaining
> programs that are designed to work with an application such as less, which
> ones purposefully close the input stream to less, and continue running?

The one run into all the time is:

rgrep "something" . | less

If you do a search and it doesn't find anything, it'll keep reading in
data until it finds it or reaches the end. Hit Ctrl-C to abort the
search and you kill the rgrep too.

Now, arguably rgrep is not "designed to work with an application such
as less" so doesn't meet your criteria. So the argument is about
whether psql is. Is the pager an integral part of the output mechanism
of psql, or just a bolt-on like a shell pipe? I suspect you can argue
either way, which is why I'm trying to find a way to have it both ways.

> SIG_IGN is the easiest, and the most effective. Certainly the easiest
> to maintain, and the least intrusive to the core.

Yeah, but doesn't fix the memory leaks and other random leakage and
breakage from the longjmp(). But I'm getting there...

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

mark@mark.mielke.cc wrote:
> On Thu, Oct 20, 2005 at 03:42:10PM -0700, Kevin Brown wrote:
> > Martijn van Oosterhout wrote:
> > > You can't do a pclose in a signal handler, it's not one of the
> > > "reeentrant safe" functions and could lead to deadlocks. The signal
> > > manpage documents the ones you can use. Just set a flag. Setting the
> > > descriptor to NULL is worse because then we have check before every
> > > output function. fprintf(NULL, ...) will segfault on most
> > > architechtures I wager. 
> > Yeah, I was thinking that you'd do the check for the flag and invoke a
> > cleanup handler after the write() to the output file descriptor.  It's
> > not clear that you'd need to do the check anyplace else.  It's been a
> > while since I've messed with this stuff, but if I recall correctly,
> > the write() will return immediately after receipt of a signal, and
> > will indicate how much was actually written.  So receipt of a SIGINT
> > should wind up being handled in a reasonably timely fashion.
> 
> > Additionally the normal SIGINT signal handler (the one that gets
> > invoked when the pager is turned off) can be called from the cleanup
> > handler in order to maintain the proper semantics.
> 
> I disagree that psql should make *any* assumptions about what SIGINT
> means to the child process. Consider less again, and Control-C used
> to abort a search. You are suggesting that Control-C should not only
> abort the search, but should also cut off the input from less. Less
> won't die. Less will just see a terminated input stream. What has been
> gained from this? Is this intuitive behaviour?

It's behaviour that's consistent with every other pipeline application
set I've ever seen.

The only difference between those and this situation is that for a
standard pipeline set, SIGINT will kill all the processes in the
pipeline.  Less explicitly ignores (or traps) SIGINT, so the effect of
generating a SIGINT where less is at the end of the pipeline is to
kill everything that precedes it, and less will then show what results
it received.  And obviously, that implies that the pipeline gets
closed.

If psql does not close the pipeline right then and there, then its
behaviour will obviously be different from what the user likely
expects, based on other pipelined uses of less.  After all, if they
wanted to see the entire result set then they wouldn't have sent
SIGINT, would they?

> If the pager does die in response to SIGINT, the write() will fail with
> SIGPIPE. Completely clean, without any need for psql to pay attention
> to SIGINT.

We're not talking about the semantics of the pager, we're talking
about the semantics of psql.  You said it yourself: psql can't make
any assumptions about what SIGINT means to the child process.  So it
has to consider what to do if the child does *not* die in response to
SIGINT.  What are the proper semantics for psql + the child in that
situation?

Well, I think it's clear that having psql ignore SIGINT in that
situation is *not* correct, because it implies that whether or not
SIGINT causes processing to stop (as the user would expect) depends
entirely on the child.  More specifically, it would depend on the
child either dying or explicitly closing its stdin upon receipt of
SIGINT.

The bottom line is that, at least in my opinion, the semantics of
SIGINT as regards psql should be the same whether or not there's a
pager involved, with one crucial difference: if there's a pager
involved, psql should wait for it to terminate before showing the
prompt.  But otherwise, the semantics should be identical, because
they are what the user expects.

> I think the only reasonable behaviour is to ignore SIGINT within the
> parent, until the child exits. I don't see why other behaviours are
> even being considered. To me, it points at a misunderstanding of the
> problem.

Not at all.  When you send SIGINT to a process, you want that process
to stop doing whatever it's doing and return control to you.  That's
what it means, and that's what it's for.  If we ignore SIGINT then
obviously we will *not* be heeding the wishes of the user who sends
SIGINT, and that is not likely what the user expects.


-- 
Kevin Brown                          kevin@sysexperts.com

Andrew - Supernews wrote:
> On 2005-10-19, Kevin Brown <kevin@sysexperts.com> wrote:
> > Making assumptions about what the pager will do upon receipt of SIGINT
> > is folly as well.
> >
> > Setting up SIGINT to be ignored may be the right answer (I don't
> > believe it is -- see below), but if so then it needs to be done
> > properly.  If it gets ignored prior to the popen(), then the child
> > will also end up ignoring it by default, because signal disposition is
> > inherited by child processes.  If we ignore SIGINT, it should be after
> > the popen(), not before.
> 
> I do not believe it is possible to do the signal disposition correctly
> and still use popen() to run the pager. (You would need to reimplement
> popen using raw syscalls.)

I'm not sure I see why this is so.  popen() just creates the pipeline,
fork()s, closes the proper file descriptor (depending on whether it's
in the parent or the child and whether the pipe was open for read or
write) and then exec()s in the child.  In the parent, it returns
control after the fork() and file descriptor cleanup.  So the parent
can set up its own internal signal disposition immediately after
popen() returns.  This sequence of events is exactly what we'd end up
doing if we did everything ourselves using raw syscalls, save for the
use of stdio instead of direct syscalls for the file operations.


> > So I think the right answer here is for psql to handle SIGINT
> > internally by doing a pclose() first
> 
> The chances that psql can do this safely approach zero. pclose() is not a
> signal-safe function, so it can only be called from a signal handler if
> you _know_ that the signal did not interrupt any non-signal-safe function.
> (Nor can the signal handler longjmp out in such a case, unless the code is
> never again going to call any unsafe function.)

I agree.  I guess I need to be a little more explicit about what I
envision here.

There would be two possible signal handlers.  The first is the one we
have now, which cleans up various things upon receipt of SIGINT.  The
second simply sets a flag that says that SIGINT has been received.

The signal handler that gets assigned to SIGINT depends on whether or
not a pager is going to be used.  If it's not, then we point it to the
first signal handler.  If it is, then we point it to the second, and
clear the flag.

When a pager is being used, we check for the flag immediately after
doing a write()/fwrite() to the pipe.  If it's set, we pclose(), clear
the flag, and then manually invoke the non-pager signal handler.
SIGINT should cause the write() to return immediately, possibly with
EINTR.


Make sense?



-- 
Kevin Brown                          kevin@sysexperts.com

If you send a recent version of vim a CONTROL-C, and you're just sitting
there at a prompt, it gives you a hint:

Type  :quit<Enter>  to exit Vim

Any reason not to just trap the CONTROL-C in psql when paging and offer a
hint? Especially since we don't really know that the user really wanted to
type CONTROL-C instead of q for quit. I know that I have always meant to
type q and was just distracted whenever I've typed CONTROL-C in the pager,
and so passing the CONTROL-C on to less is not actually "heeding my wishes",
it is instead giving me enough rope to shoot myself in the foot.

Sean

(and mixed metaphors really make by hair boil)


[big snippage]

> Not at all.  When you send SIGINT to a process, you want that process
> to stop doing whatever it's doing and return control to you.  That's
> what it means, and that's what it's for.  If we ignore SIGINT then
> obviously we will *not* be heeding the wishes of the user who sends
> SIGINT, and that is not likely what the user expects.
>
>
> -- 
> Kevin Brown       kevin@sysexperts.com
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: explain analyze is your friend
>
>

I failed to mention that I also tend to type CONTROL-C when I forget that
putty acts like an xterm, and doesn't need CONTROL-C to copy text into the
clipboard. In that case, aborting the pager, and leaving the terminal
trashed requiring me to exit psql, stty sane, and start up psql again is
really not "heeding my wishes", since what I was trying to do was copy some
text from the screen in a moment of brain-deadness.

Sean

Sean Utt wrote:
> 
> If you send a recent version of vim a CONTROL-C, and you're just sitting
> there at a prompt, it gives you a hint:
> 
> Type  :quit<Enter>  to exit Vim
> 
> Any reason not to just trap the CONTROL-C in psql when paging and offer a
> hint? Especially since we don't really know that the user really wanted to
> type CONTROL-C instead of q for quit. I know that I have always meant to
> type q and was just distracted whenever I've typed CONTROL-C in the pager,
> and so passing the CONTROL-C on to less is not actually "heeding my wishes",
> it is instead giving me enough rope to shoot myself in the foot.

It won't work properly that way.  SIGINT gets sent to all the members
of the process group, not just the child.  Psql isn't responsible for
sending ctrl-c through to the child.

When you're in an editor such as vi that makes use of the terminal,
the editor itself is likely the only program that is doing anything.
Its parent is doing a wait() on the editor.  The parent in that
instance can ignore SIGINT because it's not involved at all at that
point.

That's not the case here.  Psql and the pager are really two
cooperating parts of the same task.  They just happen to be running in
two different process spaces.  Because they're both cooperatively
active at the same time, they both need to handle SIGINT, because when
the user invokes SIGINT, he intends for the overall task to return
some kind of control to him.  For psql, which is gathering data and
sending it to the pager, that means that it needs to stop doing so and
wait for the pager to finish.  For the pager, it means at a minimum
that it needs to display what it has so far and give interactive
control to the user (it may or may not attempt to continue to read
what's being sent to it).  Some pagers (like "more") will just exit.



-- 
Kevin Brown                          kevin@sysexperts.com

On Fri, Oct 21, 2005 at 05:28:49PM -0700, Kevin Brown wrote:
> When a pager is being used, we check for the flag immediately after
> doing a write()/fwrite() to the pipe.  If it's set, we pclose(), clear
> the flag, and then manually invoke the non-pager signal handler.
> SIGINT should cause the write() to return immediately, possibly with
> EINTR.

You wish. PostgreSQL uses BSD signal semantics, which means system
calls get restarted. Neither read nor write will return when user
presses Ctrl-C... Hence my question about POSIX signals...

It doesn't matter though, if write blocks there's no processing
happening anyway and we can check the flag after write returns success
(pager accepted more data) or failure (pager died).

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

On Sun, Oct 16, 2005 at 03:25:49PM +0200, Martijn van Oosterhout wrote:
> This behaviour has been around so long that I've gotten used to it but
> I've always considered it a bug. Yet it has never been fixed so I'm
> going to ask if anybody else has issues with this behaviour.

I've posted a patch to -patches which fixes all the memory leak and
file descriptor leak issues and well as making psql handle ^C more
gracefully in general. It doesn't address this particular issue though,
thats for another time.

If people want to try it out, please do. it will appear in the archives
soon, otherwise you can download it directly from:

http://svana.org/kleptog/pgsql/psql-ctrlc.patch

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

On Sat, Oct 22, 2005 at 09:46:32PM +0200, Martijn van Oosterhout wrote:
> I've posted a patch to -patches which fixes all the memory leak and
> file descriptor leak issues and well as making psql handle ^C more
> gracefully in general. It doesn't address this particular issue though,
> thats for another time.

Clarification, it does fix the screen corruption you currently get when
you press Ctrl-C while running less. what I meant it that it preserves
current semantics w.r.t. aborting output if you press ctrl-c even
though the pager is still running.

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

> It won't work properly that way.  SIGINT gets sent to all the members
> of the process group, not just the child.  Psql isn't responsible for
> sending ctrl-c through to the child.



Except that if I am in less, and I do CONTROL-C, it doesn't do anything at 
all.

It doesn't exit.

If I send a kill -2 to the process, it doesn't exit. less ignores SIGINT 
completely.

So, whether or not they are working together in cooperation, the result we 
have been seeing when typing CONTROL-C is completely under the control of 
psql.

I do realize that the problem is one of when and how to cancel the query if 
CONTROL-C is pressed.
If the query is still building and the output hasn't, or won't be, sent to 
the pager, cancel the query.
If we are in the pager, don't respond to CONTROL-C, and instead output a 
helpful hint telling people to use q to quit, which will do what they really 
wanted anyway.
In theory, we already deal gracefully with q being pressed in the pager.

Sean

On Fri, Oct 21, 2005 at 05:06:45PM -0700, Kevin Brown wrote:
> > I disagree that psql should make *any* assumptions about what SIGINT
> > means to the child process. Consider less again, and Control-C used
> > to abort a search. You are suggesting that Control-C should not only
> > abort the search, but should also cut off the input from less. Less
> > won't die. Less will just see a terminated input stream. What has been
> > gained from this? Is this intuitive behaviour?
> It's behaviour that's consistent with every other pipeline application
> set I've ever seen.

I think you are looking at 'every other pipeline application that does
nothing', and treating them as role models. :-)

I'm completely writing off applications that don't handle SIGINT in some
way, as psql already does this, and wouldn't need *any* change to work
this way. Psql already works the way you are saying every other application
you know of does. So, I'm not seeing your point.

If psql is to change, it is because the current behaviour is impractical,
and not for any other reason. What is the practical behaviour?

I think there are other, better ways, to know that the pager is
dead. One is SIGPIPE/EPIPE. A write to the pipe to the pager will
error with EPIPE, and throw a SIGPIPE signal. The parent will also
be notified with SIGCHILD when the child exits. In this way, we
have two ways of knowing that the child is dead, without preventing
SIGINT from being handled fully by the child, in a pager such as
less, that defines SIGINT to mean something *different* than die
or abort reading the input stream.

The case that this would be a problem for me is - psql is outputting
pages and pages of data. The pipe buffer becomes full (8192 bytes on
Linux?), and psql blocks. The rest of the data is available, however,
it has not yet been transferred to less. The user does some operation
in less that might take some time, and then hits control-C. Less
continues, but under your scheme, is faced with a terminated input
stream. I'm not seeing why this is practical. I don't see the benefit
of this.

Other than the argument that other applications don't handle Control-C
at all, and you think this is compatible with their behaviour, do you
have any practical arguments for doing it this way? How is it working
better than it does today? Why won't we have the same situation of
less and psql both waiting for STDIN from the user exist after your
suggestion to make psql close off the input stream sooner?

I might just be really dense. But I want you to show me where I'm wrong
before I give in. :-)

Cheers,
mark

-- 
mark@mielke.cc / markm@ncf.ca / markm@nortel.com     __________________________
.  .  _  ._  . .   .__    .  . ._. .__ .   . . .__  | Neighbourhood Coder
|\/| |_| |_| |/    |_     |\/|  |  |_  |   |/  |_   | 
|  | | | | \ | \   |__ .  |  | .|. |__ |__ | \ |__  | Ottawa, Ontario, Canada
 One ring to rule them all, one ring to find them, one ring to bring them all                      and in the darkness
bindthem...
 
                          http://mark.mielke.cc/

On Sat, Oct 22, 2005 at 02:48:53PM -0700, Sean Utt wrote:
> Except that if I am in less, and I do CONTROL-C, it doesn't do anything at
> all.
>
> It doesn't exit.
>
> If I send a kill -2 to the process, it doesn't exit. less ignores SIGINT
> completely.

Not quite, It interprets it as "abort command". Obviously if you're not
doing anything at the time it appears to do nothing. Start a search (/)
or turn on follow mode (F) and you'll see you can (or indeed in the
latter case, must) use Ctrl-C to get back to the command prompt.

Indeed, in follow mode it says:

Waiting for data... (interrupt to abort)

> If we are in the pager, don't respond to CONTROL-C, and instead output a
> helpful hint telling people to use q to quit, which will do what they
> really wanted anyway.

That's wrong too, who says q exits the pager? Not to mention the
message screws the screen also, needing a ^L to redraw the screen. Best
do nothing at all (ie ignore). If the user doesn't know how to use
less, then they shouldn't use it as pager. Besides, less comes with
online help (press h), with how to quit being the first thing you read.

> In theory, we already deal gracefully with q being pressed in the pager.

Yes, by not reading stdin. Ergo, we should also not respond to ^C
either.

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

This has been saved for the 8.2 release:
http://momjian.postgresql.org/cgi-bin/pgpatches_hold

---------------------------------------------------------------------------

Martijn van Oosterhout wrote:
-- Start of PGP signed section.
> On Sun, Oct 16, 2005 at 04:06:04PM -0400, Andrew Dunstan wrote:
> > Martin,
> > 
> > Let's see the patch. I assume it should be fairly small. If we could get 
> > it in early in the 8.2 cycle we would have plenty of time to bang on it. 
> > In principle this sounds reasonable to me, but psql can be broken quite 
> > easily - I know as I've done it a couple of times ;-)
> 
> Very well, patch attached. It's quite simple actually. However, there
> seems to be some push back from people suggesting that jumping back to
> the main loop before the pager has quit is not buggy behaviour.
> Assuming that a ^C will kill the pager is just folly.
> 
> Tom asked if we should be blocking SIGQUIT and SIGHUP too. Standard
> procedure for spawning external interactive processes includes blocking
> SIGQUIT too (see system() manpage). Logically speaking, when the user
> sends an interrupt from the keyboard they expect to interrupt the
> currently active *interaxtive* process. Hence, once psql has spawned
> the pager, it should ignore such interrupts until control is returned
> (after pclose). So yes, I would suggest blocking SIGQUIT also, if only
> to prevent terminal corruption problems. Interactive programs like less
> trap SIGQUIT to restore the terminal settings on exit, but the exit
> anyway.
> 
> SIGHUP is different. It is sent by the system, not the user, indicating
> the terminal has disconnected. psql is not a daemon expecting to
> survive that and hence should quit as normal, along with all other
> processes on that terminal.
> 
> If this isn't going to make 8.1 anyway I'd probably go for a patch that
> got rid of the longjmp altogether (if people will accept that), fixing
> the memory leaks at the same time. At the moment I'm just looking for a
> concensus that it's a problem to be solved.
> 
> Have a nice day,
> -- 
> Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> > Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> > tool for doing 5% of the work and then sitting around waiting for someone
> > else to do the other 95% so you can sue them.

[ Attachment, skipping... ]
-- End of PGP section, PGP failed!

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073
 

On Mon, Oct 24, 2005 at 08:20:07AM -0400, Bruce Momjian wrote:
>
> This has been saved for the 8.2 release:
>
>     http://momjian.postgresql.org/cgi-bin/pgpatches_hold

Is this official "blessing" for the idea that psql should ignore SIGINT
while the pager is running? Or does this mean the idea will be
revisited again later?

If the former, I'll rework it into my psql fixing patch on -patches,
since this one as posted is not 100% correct (as someone pointed out)
although the chance that someone will notice is about nil.

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

Martijn van Oosterhout wrote:

>On Mon, Oct 24, 2005 at 08:20:07AM -0400, Bruce Momjian wrote:
>  
>
>>This has been saved for the 8.2 release:
>>
>>    http://momjian.postgresql.org/cgi-bin/pgpatches_hold
>>    
>>
>
>Is this official "blessing" for the idea that psql should ignore SIGINT
>while the pager is running? Or does this mean the idea will be
>revisited again later?
>
>
>  
>
AFAIK it's Bruce's way of not losing track of the patch, so more a case 
of "revisit later" than "officially blessed".

By all means submit a revised patch if you improve on it in the meantime.

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:
> Martijn van Oosterhout wrote:
>> On Mon, Oct 24, 2005 at 08:20:07AM -0400, Bruce Momjian wrote:
>>> This has been saved for the 8.2 release:

>> Is this official "blessing" for the idea

> AFAIK it's Bruce's way of not losing track of the patch,

Exactly.  "Held for future review" would be a more accurate reading.
        regards, tom lane

Modified version of this patch applied by Tom.

---------------------------------------------------------------------------

Martijn van Oosterhout wrote:
-- Start of PGP signed section.
> On Sun, Oct 16, 2005 at 04:06:04PM -0400, Andrew Dunstan wrote:
> > Martin,
> > 
> > Let's see the patch. I assume it should be fairly small. If we could get 
> > it in early in the 8.2 cycle we would have plenty of time to bang on it. 
> > In principle this sounds reasonable to me, but psql can be broken quite 
> > easily - I know as I've done it a couple of times ;-)
> 
> Very well, patch attached. It's quite simple actually. However, there
> seems to be some push back from people suggesting that jumping back to
> the main loop before the pager has quit is not buggy behaviour.
> Assuming that a ^C will kill the pager is just folly.
> 
> Tom asked if we should be blocking SIGQUIT and SIGHUP too. Standard
> procedure for spawning external interactive processes includes blocking
> SIGQUIT too (see system() manpage). Logically speaking, when the user
> sends an interrupt from the keyboard they expect to interrupt the
> currently active *interaxtive* process. Hence, once psql has spawned
> the pager, it should ignore such interrupts until control is returned
> (after pclose). So yes, I would suggest blocking SIGQUIT also, if only
> to prevent terminal corruption problems. Interactive programs like less
> trap SIGQUIT to restore the terminal settings on exit, but the exit
> anyway.
> 
> SIGHUP is different. It is sent by the system, not the user, indicating
> the terminal has disconnected. psql is not a daemon expecting to
> survive that and hence should quit as normal, along with all other
> processes on that terminal.
> 
> If this isn't going to make 8.1 anyway I'd probably go for a patch that
> got rid of the longjmp altogether (if people will accept that), fixing
> the memory leaks at the same time. At the moment I'm just looking for a
> concensus that it's a problem to be solved.
> 
> Have a nice day,
> -- 
> Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> > Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> > tool for doing 5% of the work and then sitting around waiting for someone
> > else to do the other 95% so you can sue them.

[ Attachment, skipping... ]
-- End of PGP section, PGP failed!

--  Bruce Momjian   http://candle.pha.pa.us EnterpriseDB    http://www.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +