From: | Martijn van Oosterhout <kleptog(at)svana(dot)org> |
---|---|
To: | pgsql-patches(at)postgresql(dot)org |
Subject: | [PATCH] Fix Ctrl-C related issues in psql (not for 8.1) |
Date: | 2005-10-22 19:42:27 |
Message-ID: | 20051022194221.GE16589@svana.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-patches |
[Please CC any replies to me, thanks. Also, the actual discussion took
place on -hackers, so check there first]
As anyone who has been following -hackers knows, there's been a bit of
discussion about how psql should deal with ^C and pagers. Attached is a
patch that deals with all the memory leak and descriptor leak issues.
With this patch, even valgrind can't find any leaked memory (except a
few things from program startup). With this patch, I find psql a
pleasure to use, it doesn't bug me at all anymore.
As for the apparent complexity of the patch, I'll quote the new comment
from common.c.
* About ctrlc_abort_jmp and ctrlc_abort_active:
*
* In the past the SIGINT handler, when it couldn't find a query to cancel,
* would simply longjmp() back to the main loop. Very simple but
* unfortunatly it leaked memory like crazy and file descriptors too if you
* were using a pager. Hence, the handler has been changed to just set a
* flag in those cases so the code would notice and gracefully quit.
*
* However, PostgreSQL uses signals with BSD semantics, which means the
* system calls get restarted across a signal. Hence when sitting at the
* prompt pressing Ctrl-C appeared to do nothing because the kernel would
* keep restarting the read(). The only way to avoid this is to have the
* signal handler longjmp().
*
* Hence the rules are as follows: If you are reading from a descriptor
* which may block indefinitly and you want to be able to respond to Ctrl-C
* you should setup with sigsetjmp() the jump buffer ctrlc_abort_jmp. And
* when you are about to read something set ctrlc_abort_active to true.
* After the read set it back to false. If while the flag is true the user
* presses Ctrl-C, control will pass to the sigsetjmp() block which allows
* you to clean up gracefully and return. See gets_basic() and
* gets_readline() in input.c for examples.
Yes, SysV semantics for signals would simplify the code a bit, but I
have no idea whether that's supported or not. Certainly there isn't any
support for it now in the postgres codebase.
There are minor cosmetic changes, like a message when you interrupted
the processing of a query. If you interrupt a pager which dies on ^C
(like more), psql stops generating output straight away instead of
continuing.
I've tested as much as I can think of and it handles everything nicely.
The question of ignoring SIGINT while the pager is active is
orthoginal, this patch doesn't address that at all.
Any comments on coding style and/or side-effects of this patch, don't
hesitate to forward to me.
Also available at:
http://svana.org/kleptog/pgsql/psql-ctrlc.patch
Have a nice day,
--
Martijn van Oosterhout <kleptog(at)svana(dot)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 | Content-Type | Size |
---|---|---|
psql-ctrlc.patch | text/plain | 23.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Greg Sabino Mullane | 2005-10-23 21:36:59 | Re: suggested warning about perl nested named subroutines |
Previous Message | Andrew Dunstan | 2005-10-22 14:28:32 | Re: small code cleanup - gettimeofday() |