Re: [HACKERS] Question about Ctrl-C and less
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 EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] Question about Ctrl-C and less
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, Pennsylvania 19073 ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Question about Ctrl-C and less
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. pgpyhBOAofWiB.pgp Description: PGP signature
Re: [HACKERS] Question about Ctrl-C and less
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 ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] Question about Ctrl-C and less
Andrew Dunstan [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] Question about Ctrl-C and less
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. pgpEMzGRlaosM.pgp Description: PGP signature
Re: [HACKERS] Question about Ctrl-C and less
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 :quitEnter 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 [EMAIL PROTECTED] ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Question about Ctrl-C and less
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. pgpRIOQrVoQT9.pgp Description: PGP signature
Re: [HACKERS] Question about Ctrl-C and less
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. pgplcAHg8WEVu.pgp Description: PGP signature
Re: [HACKERS] Question about Ctrl-C and less
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. pgpLrIwaLBQtv.pgp Description: PGP signature
Re: [HACKERS] Question about Ctrl-C and less
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 ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] Question about Ctrl-C and less
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 -- [EMAIL PROTECTED] / [EMAIL PROTECTED] / [EMAIL PROTECTED] __ . . _ ._ . . .__. . ._. .__ . . . .__ | 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 bind them... http://mark.mielke.cc/ ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Question about Ctrl-C and less
On Thu, Oct 20, 2005 at 08:11:14PM -0400, [EMAIL PROTECTED] 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. pgpaRH7VBVICN.pgp Description: PGP signature
Re: [HACKERS] Question about Ctrl-C and less
On Fri, Oct 21, 2005 at 01:53:32PM +0200, Martijn van Oosterhout wrote: On Thu, Oct 20, 2005 at 08:11:14PM -0400, [EMAIL PROTECTED] 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 -- [EMAIL PROTECTED] / [EMAIL PROTECTED] / [EMAIL PROTECTED] __ . . _ ._ . . .__. . ._. .__ . . . .__ | 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 bind them... http://mark.mielke.cc/ ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] Question about Ctrl-C and less
On Fri, Oct 21, 2005 at 08:48:31AM -0400, [EMAIL PROTECTED] 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. pgpe84NmsO9L6.pgp Description: PGP signature
Re: [HACKERS] Question about Ctrl-C and less
[EMAIL PROTECTED] 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 [EMAIL PROTECTED] ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] Question about Ctrl-C and less
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 :quitEnter 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 [EMAIL PROTECTED] ---(end of broadcast)--- TIP 6: explain analyze is your friend ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] Question about Ctrl-C and less
Andrew - Supernews wrote: On 2005-10-19, Kevin Brown [EMAIL PROTECTED] 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 [EMAIL PROTECTED] ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] Question about Ctrl-C and less
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 ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Question about Ctrl-C and less
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 [EMAIL PROTECTED] ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] Question about Ctrl-C and less
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 -- [EMAIL PROTECTED] / [EMAIL PROTECTED] / [EMAIL PROTECTED] __ . . _ ._ . . .__. . ._. .__ . . . .__ | 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 bind them... http://mark.mielke.cc/ ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] Question about Ctrl-C and less
On 2005-10-19, Kevin Brown [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] Question about Ctrl-C and less
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. ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] Question about Ctrl-C and less
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. pgpaHwIYul74p.pgp Description: PGP signature
Re: [HACKERS] Question about Ctrl-C and less
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 [EMAIL PROTECTED] ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] Question about Ctrl-C and less
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 ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Question about Ctrl-C and less
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. pgpPHzuuSbHDW.pgp Description: PGP signature
Re: [HACKERS] Question about Ctrl-C and less
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 ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] Question about Ctrl-C and less
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 ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] Question about Ctrl-C and less
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. ? gmon.out ? psql Index: print.c === RCS file: /projects/cvsroot/pgsql/src/bin/psql/print.c,v retrieving revision 1.77 diff -u -r1.77 print.c --- print.c 4 Oct 2005 19:01:18 - 1.77 +++ print.c 16 Oct 2005 21:14:45 - @@ -1448,6 +1448,7 @@ pagerprog = DEFAULT_PAGER; #ifndef WIN32 pqsignal(SIGPIPE, SIG_IGN); + pqsignal(SIGINT, SIG_IGN); #endif return popen(pagerprog, w); #ifdef TIOCGWINSZ @@ -1589,6 +1590,7 @@ pclose(output); #ifndef WIN32 pqsignal(SIGPIPE, SIG_DFL); + pqsignal(SIGINT, handle_sigint); #endif } } pgpEjb3y6AVqZ.pgp Description: PGP signature
Re: [HACKERS] Question about Ctrl-C and less
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 -- [EMAIL PROTECTED] / [EMAIL PROTECTED] / [EMAIL PROTECTED] __ . . _ ._ . . .__. . ._. .__ . . . .__ | 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 bind them... http://mark.mielke.cc/ ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
