Re: [ccache] ccache interrupt handling bug

2015-08-30 Thread Nadav Har'El
On Sat, Aug 29, 2015 at 8:01 PM, Joel Rosdahl j...@rosdahl.net wrote:

 On 20 August 2015 at 21:04, Joel Rosdahl j...@rosdahl.net wrote:

 I will not have time to work on improving the signal handler until next
 week at the earliest.


 I've now had a look at improving the signal handler. It would be
 appreciated if somebody wants to review it:
 https://git.samba.org/?p=ccache.git;a=commitdiff;h=737c96fabeab9d66407758b848468aa49190c946


Hi,

Looking at this code, I don't see any obvious holes and it all seems
correct.

One thing you should perhaps consider is whether this change (using
sigaction, signal sets, instead of the antique signal(), etc.) might break
compilation on some antique UNIX machines or on MS-Windows (!?) or
something. But I doubt any present-day system actually lacks sigaction()
and friends, so I think your code is good.




-- 
Nadav Har'El
n...@cloudius-systems.com
___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache


Re: [ccache] ccache interrupt handling bug

2015-08-30 Thread Tom Lane
Nadav Har'El n...@cloudius-systems.com writes:
 One thing you should perhaps consider is whether this change (using
 sigaction, signal sets, instead of the antique signal(), etc.) might break
 compilation on some antique UNIX machines or on MS-Windows (!?) or
 something. But I doubt any present-day system actually lacks sigaction()
 and friends, so I think your code is good.

Windows is definitely a risk factor here: AFAIK they don't provide a
decent emulation of the POSIX signal functions.  However, they don't
have the BSD ones either, so I assume ccache is already covering
Windows specially.

As for the antique UNIX angle, I can offer a data point: Postgres
still nominally carries support for the pre-POSIX signal API, but
AFAICT that code isn't being used on any platform still in use.
*All* of the active non-Windows members of our buildfarm report that
their configure test for POSIX signals succeeds.  And there are some
pretty darn old systems in there:
http://buildfarm.postgresql.org/cgi-bin/show_status.pl

regards, tom lane

___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache


Re: [ccache] ccache interrupt handling bug

2015-08-29 Thread Joel Rosdahl
On 20 August 2015 at 21:04, Joel Rosdahl j...@rosdahl.net wrote:

 I will not have time to work on improving the signal handler until next
 week at the earliest.


I've now had a look at improving the signal handler. It would be
appreciated if somebody wants to review it:
https://git.samba.org/?p=ccache.git;a=commitdiff;h=737c96fabeab9d66407758b848468aa49190c946

-- Joel
___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache


Re: [ccache] ccache interrupt handling bug

2015-08-20 Thread Joel Rosdahl
On 19 August 2015 at 15:45, Mike Frysinger vap...@gentoo.org wrote:

this wouldn't solve things entirely.  calling _exit(1) is fundamentally
 wrong. here's a pretty good page giving a rundown: [...]


Interesting, thank you. And thanks to Nadav and Eitan as well for good
input.

I will not have time to work on improving the signal handler until next
week at the earliest. If anybody who is more knowledgeable than I currently
am about signal handlers want to prepare patches or so, that would be very
welcome.

-- Joel
___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache


Re: [ccache] ccache interrupt handling bug

2015-08-16 Thread Tom Lane
Nadav Har'El n...@cloudius-systems.com writes:
 First of all, thanks. This indeed fixes the bug, and is exactly the first
 patch I tried to fix it this problem.

 I want to explain, though, why I ended up sending a different patch for
 this problem - a 4-line patch instead of this one-liner. The explanation is
 not very important, and I should probably have done this earlier, but
 perhaps someone would find it interesting.

 When the user types ^C, the operating system sends SIGINT to all processes
 belonging to the terminal's process group - and in particular it sends
 SIGINT to *both* the child compiler and ccache, separately.

 Your patch makes ccache exit as soon as it gets the SIGINT - but the child
 compiler might still be running for a while longer. This will usually be
 fine, but I thought the user can be surprised if he sees ccache (which he
 considers to be the compiler) exit, but ps shows the compiler is still
 running. This would be especially annoying if some hypothetical compiler
 trapped SIGINT deliberately, and continued to run long after ccache exits.

Actually, that's a bug not just a cosmetic problem, because it introduces
a race condition.  Consider this sequence of events:

* make launches ccache gcc -o test.o test.c
* user types control-C
* ccache receives SIGINT and exits
* make does unlink(test.o) to clean up after failed compile step, which
  it thinks is done
* gcc creates test.o
* gcc receives SIGINT and exits

We're left with a probably-broken test.o in the filesystem, despite
make's attempt to clean up.  Now, the window for this is pretty narrow,
but I think it could happen: I don't believe that sending signals to
multiple processes is atomic in most kernels.

You really don't want ccache to exit until after all externally-observable
things have stopped happening.  (I recently fixed a somewhat related bug
in Postgres :-(, which is why this caught my eye.)

regards, tom lane

___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache


Re: [ccache] ccache interrupt handling bug

2015-08-16 Thread Tom Lane
Nadav Har'El n...@cloudius-systems.com writes:
 On Mon, Aug 17, 2015 at 12:50 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 * make launches ccache gcc -o test.o test.c
 * user types control-C
 * ccache receives SIGINT and exits
 * make does unlink(test.o) to clean up after failed compile step, which
 it thinks is done
 * gcc creates test.o

 Stupid question: Does ccache let gcc write to test.o directly, or does it
 write to some temporary file and then ccache copies it?

Dunno, but that just moves the problem somewhere else no?  For instance,
in the code fragment you mentioned, I wonder whether it's sane to do
clean_up_pending_tmp_files(); when gcc is still on the loose.

regards, tom lane

___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache


Re: [ccache] ccache interrupt handling bug

2015-08-16 Thread Nadav Har'El
On Sun, Aug 9, 2015 at 11:23 PM, Joel Rosdahl j...@rosdahl.net wrote:

 Hi Nadav,

 (Sorry for the delayed reply.)

 Hi, I found a bug in ccache, which makes it impossible to
 correctly interrupt a compilation with a control-C (I tried this on Linux).
 [...]


 Thanks for the bug report and analysis. This will be fixed in ccache 3.2.3.

 Regards,
 -- Joel


Thanks. I see now the fix in 3.2.3, which if I understand correctly, is
simply:

--- a/ccache.c
+++ b/ccache.c
@@ -341,6 +341,7 @@ signal_handler(int signo)
 {
(void)signo;
clean_up_pending_tmp_files();
+   _exit(1);
 }

First of all, thanks. This indeed fixes the bug, and is exactly the first
patch I tried to fix it this problem.

I want to explain, though, why I ended up sending a different patch for
this problem - a 4-line patch instead of this one-liner. The explanation is
not very important, and I should probably have done this earlier, but
perhaps someone would find it interesting.

When the user types ^C, the operating system sends SIGINT to all processes
belonging to the terminal's process group - and in particular it sends
SIGINT to *both* the child compiler and ccache, separately.

Your patch makes ccache exit as soon as it gets the SIGINT - but the child
compiler might still be running for a while longer. This will usually be
fine, but I thought the user can be surprised if he sees ccache (which he
considers to be the compiler) exit, but ps shows the compiler is still
running. This would be especially annoying if some hypothetical compiler
trapped SIGINT deliberately, and continued to run long after ccache exits.

My approach, was to let ccache continue after the SIGINT, and continue to
wait (as it always does) for the child compiler to finish. The child, who
received a SIGINT too, willl indeed finish quickly. The code already
recognizes this case of the child killed by a signal, and returns status ==
-1. So all my 4-line patch did was to not restart the compiler if it died
with a status = -1 (i.e., a signal) and not an ordinary failure. But ccache
does not exit immediately on a signal - just after the child compiler exits.

Again, I guess this destinction is not very important, but I just wanted to
point it out in case anybody thinks it is important.

-- 
Nadav Har'El
n...@cloudius-systems.com
___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache


Re: [ccache] ccache interrupt handling bug

2015-08-16 Thread Nadav Har'El
On Mon, Aug 17, 2015 at 12:50 AM, Tom Lane t...@sss.pgh.pa.us wrote:


 * make launches ccache gcc -o test.o test.c
 * user types control-C
 * ccache receives SIGINT and exits
 * make does unlink(test.o) to clean up after failed compile step, which
   it thinks is done
 * gcc creates test.o


Stupid question: Does ccache let gcc write to test.o directly, or does it
write to some temporary file and then ccache copies it?


 * gcc receives SIGINT and exits

 We're left with a probably-broken test.o in the filesystem, despite
 make's attempt to clean up.  Now, the window for this is pretty narrow,
 but I think it could happen: I don't believe that sending signals to
 multiple processes is atomic in most kernels.

 You really don't want ccache to exit until after all externally-observable
 things have stopped happening.  (I recently fixed a somewhat related bug
 in Postgres :-(, which is why this caught my eye.)

 regards, tom lane




-- 
Nadav Har'El
n...@cloudius-systems.com
___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache


Re: [ccache] ccache interrupt handling bug

2015-08-09 Thread Joel Rosdahl
Hi Nadav,

(Sorry for the delayed reply.)

Hi, I found a bug in ccache, which makes it impossible to
 correctly interrupt a compilation with a control-C (I tried this on Linux).
 [...]


Thanks for the bug report and analysis. This will be fixed in ccache 3.2.3.

Regards,
-- Joel
___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache