Subject: Re: Solaris cloning woes partly diagnosed

From: Jason Riedy <[email-masked]>
Date: Sun, 02 Apr 2006 12:52:35 -0700

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");
 	}