Re: Solaris cloning woes partly diagnosed

From: Jason Riedy <ejr@eecs.berkeley.edu>
Date: Apr 2, 2006, 19:52 UTC
Message-ID: <824.1144007555@lotus.CS.Berkeley.EDU>
In-Reply-To: <Pine.LNX.4.64.0604021159110.3050@g5.osdl.org>

And Linus Torvalds writes:
 - 
 - so it really really looks like fgets() would have problems with a SIGALRM 
 - coming in and doesn't just re-try on EINTR. Can Solaris stdio _really_ be 
 - that broken? (Yeah, yeah, it may be "conforming". It's also so incredibly 
 - programmer-unfriendly that it's not even funny)

Yes, it is that broken. I haven't encountered the problem consistently in git myself, so I can't tell you if the patch works. Google finds similar reports and patches for BOINC, ruby, and a few other projects.

Solaris folks will say you should be using sigaction with SA_RESTART. IIRC, SA_RESTART isn't guaranteed to be there or work, but all the systems I deal with right now have it. So an alternate patch for this one use is appended... Other uses of signal could be changed to sigaction, too. And progress_update "should" be sig_atomic_t.

Passes the pack-objects tests, but I can't make the problem happen on demand. (I have seen it occur before, but never during make test, and I'd not tracked it down...)

Jason ----

diff --git a/pack-objects.c b/pack-objects.c
index ccfaa5f..1faa0bb 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -877,10 +877,21 @@ static int try_delta(struct unpacked *cu
 	return 0;
 }
 
-static void progress_interval(int signum)
+static void progress_interval(int);
+
+static void setup_progress_signal(void)
+{
+	struct sigaction sa;
+	sa.sa_handler = progress_interval;
+	sigemptyset(&sa.sa_mask);
+	sa.sa_flags = SA_RESTART;
+	sigaction(SIGALRM, &sa, NULL);
+}
+
+void progress_interval(int signum)
 {
-	signal(SIGALRM, progress_interval);
 	progress_update = 1;
+	setup_progress_signal();
 }
 
 static void find_deltas(struct object_entry **list, int window, int depth)
@@ -1094,7 +1105,7 @@ int main(int argc, char **argv)
 		v.it_interval.tv_sec = 1;
 		v.it_interval.tv_usec = 0;
 		v.it_value = v.it_interval;
-		signal(SIGALRM, progress_interval);
+		setup_progress_signal();
 		setitimer(ITIMER_REAL, &v, NULL);
 		fprintf(stderr, "Generating pack...\n");
 	}