thread runtime system bug

[Stephen Weeks]

I have tracked down a thread bug to the following.

Here is the relevant code:
* handler is the C signal handler
* Thread_atomicBegin is called to start a critical section
* gcState is declared with 
	extern struct GC_state gcState;

--------------------------------------------------

static void handler(Int signum) {
	GC_handler(&gcState, signum);
}

void GC_handler(GC_state s, int signum) {
	if (DEBUG)
		fprintf(stderr, "GC_handler\n");
	if (0 == s->canHandle) {
		if (DEBUG)
			fprintf(stderr, "setting limit = base\n");
		s->limit = s->base;
	}
	sigaddset(&s->signalsPending, signum);
	s->signalIsPending = TRUE;
}


void Thread_atomicBegin() {
	if (DEBUG)
		fprintf(stderr, "atomicBegin %d -> %d\n", 
				gcState.canHandle, gcState.canHandle + 1);
	assert(gcState.canHandle >= 0);
 	gcState.canHandle++;
	if (FALSE and DEBUG)
		fprintf(stderr, "atomicBegin canHandle++ done\n");
	if (gcState.signalIsPending) {
		if (DEBUG)
			fprintf(stderr, "atomicBegin signal pending\n");
		setLimit(&gcState);
	} else {
		if (DEBUG)
			fprintf(stderr, "atomicBegin no signal pending\n");
	}
	if (DEBUG)
		fprintf(stderr, "atomicBegin done\n");
}

--------------------------------------------------

When running, I see the following on stderr

atomicBegin 0 -> 1
GC_handler
setting limit = base
atomicBegin no signal pending
atomicBegin done

What this tells me is that the following sequence of events happens.
1. Thread_atomicBegin starts running
2. A signal arrives before the gcState.canHandle++ statement
3. The handler runs, sets limit to base and sets signalIsPending.
4. Thread_atomicBegin continues, but does not see that signalIsPending.

My question: is this because gcState is not declared volatile?  Should it be?
I certainly intended when I wrote the code for Thread_atomicBegin for it to work
correctly if it is interrupted by the C signal handler.