Re: PATCH: SysV semaphore race vs SIGSTOP

[Manfred Spraul <manfred@colorfullife.com>]

[-- Attachment #1: Type: text/plain, Size: 1085 bytes --]

Hi Ove,

>As I mentioned in an earlier mail, there is a race when SIGSTOP-ing a
>process waiting for a SysV semaphore, where if a process holding a
>semaphore suspends another process waiting on the semaphore and then
>releases the semaphore, 
>
Your patch looks correct (for 2.4 - 2.6 uses a different approach), but 
I'm not certain that it's needed:
You assume that signals have an immediate effect.
Linux ignores that - it delays signal processing under some 
circumstances. If a syscall can be completed without blocking, then the 
syscall is handled, regardless of any pending signals. The signal is 
handled at the syscall return, i.e. after completing the syscall.
That's not just in SysV semaphore - at least pipes are identical:
pipe_read first check if there is data. If there is some, then it 
returns the data. Signals are only checked if there is no pending data.
I'm not sure if this is a bug. But if it's one, then far more than just 
sysv sem must be updated.

What about other unices? I've attached a test app that tests pipes. 
Could someone try it?

--
    Manfred

[-- Attachment #2: test8.cpp --]
[-- Type: text/x-c++src, Size: 2024 bytes --]

/*
 * Copyright (C) 1999, 2001,2005 by Manfred Spraul.
 * 
 * Redistribution of this file is permitted under the terms of the GNU 
 * General Public License (GPL)
 * $Header: /home/manfred/cvs-tree/pipetest/test8.cpp,v 1.1 2005/02/05 08:35:01 manfred Exp $
 */

#include <unistd.h>
#include <fcntl.h>
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <signal.h>
#include <limits.h>
#include <errno.h>

#define WRITE_LEN	128	/* less than PIPE_BUF */

int buffer[WRITE_LEN];

void dummy(int sig)
{
}

int main()
{
	int pipes[2];
	int ret;

	ret = pipe(pipes);
	if(ret != 0) {
		printf("pipe creation failed, ret %d, errno %d.\n",
				ret, errno);
		exit(1);
	}
	ret = fork();
	if(!ret) {
		/* child: read from pipe */
		printf("child: trying to read %d bytes.\n", WRITE_LEN);
		ret = read(pipes[0], buffer, WRITE_LEN);
		/* never executed! */
		printf("child: read returned %d.\n", ret);
		printf("SIGSTOP test result: SIGSTOP completely broken\n");
	} else {
		/* synchronize with timer - the following block must be atomic */
		sleep(1);
		/* begin atomic block */
		ret = kill(ret, SIGSTOP);
		if (ret != 0) {
			printf("Sending SIGSTOP failed, aborting (errno=%d).\n", errno);
			exit(1);
		}
		ret = write(pipes[1], buffer, WRITE_LEN);
		if (ret != WRITE_LEN) {
			printf("Writing to pipe buffer failed, aborting (errno=%d).\n", errno);
			exit(1);
		}
		/* end of atomic block */
		printf("parent: yielding\n");
		sleep(1);
		ret = fcntl(pipes[0], F_SETFL, O_NONBLOCK);
		if (ret != 0) {
			printf("fcntl(,,O_NONBLOCK) failed, aborting (errno=%d).\n", errno);
			exit(1);
		}
		ret = read(pipes[0], buffer, WRITE_LEN);
		printf("parent: read returned %d.\n", ret);
		printf("\n\n");
		printf("SIGSTOP test result:\n");
		printf("Expected values:\n");
		printf("	%d for OS with synchroneous SIGSTOP\n", WRITE_LEN);
		printf("	-1 for OS with asynchroneous SIGSTOP (errno EAGAIN=%d)\n", EAGAIN);
		printf("Got: %d (errno: %d)\n", ret, errno);
		close(pipes[1]);
		close(pipes[0]);
	}
}