Re: [LTP] [PATCH v3 2/2] open_posix_testsuite/../mq_timedsend/12-1: fix race

[chrubis@suse.cz]

Hi!
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
> Changes in v3:
>   remove SAFE() macro
>   convert pipes to mutex and condition
> 
>  .../conformance/interfaces/mq_timedsend/12-1.c     |  175 ++++++++++++--------
>  1 files changed, 107 insertions(+), 68 deletions(-)
> 
> diff --git a/testcases/open_posix_testsuite/conformance/interfaces/mq_timedsend/12-1.c b/testcases/open_posix_testsuite/conformance/interfaces/mq_timedsend/12-1.c
> index 29780d0..fdcb6ec 100644
> --- a/testcases/open_posix_testsuite/conformance/interfaces/mq_timedsend/12-1.c
> +++ b/testcases/open_posix_testsuite/conformance/interfaces/mq_timedsend/12-1.c
> @@ -39,19 +39,23 @@
>  #define MSGSTR "0123456789"
>  #define BUFFER 40
>  #define MAXMSG 10
> +#define TIMEOUT 10		/* seconds mq_timedsend will block */
> +#define SIGNAL_DELAY_MS 50	/* delay in ms between 2 signals */
> +#define STATE_THREAD_READY 1
> +#define STATE_DONE_SENDING 2
>  
> -#define INTHREAD 0
> -#define INMAIN 1
> +#define handle_error_en(en, msg) \
> +	do { errno = en; perror(msg); exit(PTS_UNRESOLVED); } while (0)

Maybe error_and_exit() would be a bit better name, but that is very
minor.

> -/* manual semaphore */
> -int sem;
> +/* variable to indicate how many times signal handler was called */
> +static volatile sig_atomic_t in_handler;
>  
> -/* flag to indicate signal handler was called */
> -int in_handler;
> +/* errno returned by mq_timedsend() */
> +static int mq_timedsend_errno = -1;
>  
> -/* flag to indicate that errno was set to eintr when mq_timedsend()
> - * was interruped. */
> -int errno_eintr;
> +static int state;
> +pthread_mutex_t mutex;
> +pthread_cond_t cond;
>  
>  /*
>   * This handler is just used to catch the signal and stop sleep (so the
> @@ -59,12 +63,46 @@ int errno_eintr;
>   */
>  void justreturn_handler(int signo)
>  {
> -	/* Indicate that the signal handler was called */
> -	in_handler = 1;
> -	return;
> +	in_handler++;
>  }
>  
> -void *a_thread_func()
> +void set_state(int n)
> +{
> +	int ret;
> +
> +	ret = pthread_mutex_lock(&mutex);
> +	if (ret != 0)
> +		handle_error_en(ret, "set_state pthread_mutex_lock");
> +
> +	state = n;
> +	ret = pthread_cond_signal(&cond);
> +	if (ret != 0)
> +		handle_error_en(ret, "set_state pthread_cond_signal");
> +
> +	ret = pthread_mutex_unlock(&mutex);
> +	if (ret != 0)
> +		handle_error_en(ret, "set_state pthread_mutex_unlock");
> +}
> +
> +void wait_for_state(int n)
> +{
> +	int ret;
> +
> +	ret = pthread_mutex_lock(&mutex);
> +	if (ret != 0)
> +		handle_error_en(ret, "wait_for_state pthread_mutex_lock");
> +
> +	while (state != n) {
> +		ret = pthread_cond_wait(&cond, &mutex);
> +		if (ret != 0)
> +			handle_error_en(ret, "pthread_cond_wait");
> +	}
> +	ret = pthread_mutex_unlock(&mutex);
> +	if (ret != 0)
> +		handle_error_en(ret, "wait_for_state pthread_mutex_unlock");
> +}

Hmm, that gets unnecessary complicated :(

And all we need is to get first thread wait until the second is ready
and then the second until the first has finished sending signals. I
wonder if pthread_barrier or two wouldn't be easier way around.

> +void *a_thread_func(void *arg)
>  {
>  	int i, ret;
>  	struct sigaction act;
> @@ -86,101 +124,102 @@ void *a_thread_func()
>  	attr.mq_maxmsg = MAXMSG;
>  	attr.mq_msgsize = BUFFER;
>  	gqueue = mq_open(gqname, O_CREAT | O_RDWR, S_IRUSR | S_IWUSR, &attr);
> -	if (gqueue == (mqd_t) - 1) {
> -		perror("mq_open() did not return success");
> -		pthread_exit((void *)PTS_UNRESOLVED);
> -		return NULL;
> -	}
> +	if (gqueue == (mqd_t) -1)
> +		handle_error_en(errno, "mq_open");
>  
> -	/* mq_timedsend will block for 10 seconds when it waits */
> -	ts.tv_sec = time(NULL) + 10;
> +	/* mq_timedsend will block for TIMEOUT seconds when it waits */
> +	ts.tv_sec = time(NULL) + TIMEOUT;
>  	ts.tv_nsec = 0;
>  
>  	/* Tell main it can go ahead and start sending SIGUSR1 signal */
> -	sem = INMAIN;
> +	set_state(STATE_THREAD_READY);
>  
>  	for (i = 0; i < MAXMSG + 1; i++) {
>  		ret = mq_timedsend(gqueue, msgptr, strlen(msgptr), 1, &ts);
> -		if (ret	!= -1)
> +		if (ret != -1)
>  			continue;
>  
> +		mq_timedsend_errno = errno;
>  		if (errno == EINTR) {
> -			if (mq_unlink(gqname) != 0) {
> -				perror("mq_unlink() did not return success");
> -				pthread_exit((void *)PTS_UNRESOLVED);
> -				return NULL;
> -			}
> +			if (mq_unlink(gqname) != 0)
> +				handle_error_en(errno, "mq_unlink");
>  			printf("thread: mq_timedsend interrupted by signal"
>  				" and correctly set errno to EINTR\n");
> -			errno_eintr = 1;
> +			wait_for_state(STATE_DONE_SENDING);
>  			pthread_exit((void *)PTS_PASS);
> -			return NULL;
>  		} else {
>  			printf("mq_timedsend not interrupted by signal or"
>  				" set errno to incorrect code: %d\n", errno);
> +			wait_for_state(STATE_DONE_SENDING);
>  			pthread_exit((void *)PTS_FAIL);
> -			return NULL;
>  		}
>  	}
>  
> -	/* Tell main that it the thread did not block like it should have */
> -	sem = INTHREAD;
> -
> -	perror("Error: thread never blocked\n");
> +	mq_timedsend_errno = 0;
> +	printf("Error: mq_timedsend wasn't interrupted\n");
> +	wait_for_state(STATE_DONE_SENDING);
>  	pthread_exit((void *)PTS_FAIL);
> -	return NULL;
>  }
>  
>  int main(void)
>  {
>  	pthread_t new_th;
> -	int i;
> +	int i = 0, ret;
>  
> -	/* Initialized values */
> -	i = 0;
> -	in_handler = 0;
> -	errno_eintr = 0;
> -	sem = INTHREAD;
> +	ret = pthread_mutex_init(&mutex, NULL);
> +	if (ret != 0)
> +		handle_error_en(ret, "pthread_mutex_init");

You can initialize the mutex statically with PTHRED_MUTEX_INITIALIZER.

> -	if (pthread_create(&new_th, NULL, a_thread_func, NULL) != 0) {
> -		perror("Error: in pthread_create\n");
> -		return PTS_UNRESOLVED;
> -	}
> +	ret = pthread_cond_init(&cond, NULL);
> +	if (ret != 0)
> +		handle_error_en(ret, "pthread_cond_init");

Here as well with PTHREAD_COND_INIT.

> -	/* Wait for thread to set up handler for SIGUSR1 */
> -	while (sem == INTHREAD)
> -		sleep(1);
> +	ret = pthread_create(&new_th, NULL, a_thread_func, NULL);
> +	if (ret != 0)
> +		handle_error_en(ret, "pthread_create");
>  
> -	while ((i != 10) && (sem == INMAIN)) {
> +	/* wait for thread to start */
> +	wait_for_state(STATE_THREAD_READY);
> +
> +	while (i < TIMEOUT*1000 && mq_timedsend_errno < 0) {
>  		/* signal thread while it's in mq_timedsend */
> -		if (pthread_kill(new_th, SIGUSR1) != 0) {
> -			perror("Error: in pthread_kill\n");
> -			return PTS_UNRESOLVED;
> -		}
> -		i++;
> +		ret = pthread_kill(new_th, SIGUSR1);
> +		if (ret != 0)
> +			handle_error_en(ret, "pthread_kill");
> +		usleep(SIGNAL_DELAY_MS*1000);
> +		i += SIGNAL_DELAY_MS;
>  	}
>  
> -	if (pthread_join(new_th, NULL) != 0) {
> -		perror("Error: in pthread_join()\n");
> -		return PTS_UNRESOLVED;
> -	}
> +	/* signal thread we are done sending SIGUSR1 */
> +	set_state(STATE_DONE_SENDING);
> +
> +	ret = pthread_join(new_th, NULL);
> +	if (ret != 0)
> +		handle_error_en(ret, "pthread_join");
>  
>  	/* Test to see if the thread blocked correctly in mq_timedsend,
>  	 * and if it returned EINTR when it caught the signal */
> -	if (errno_eintr != 1) {
> -		if (sem == INTHREAD) {
> -			printf("Test FAILED: mq_timedsend() never"
> -				" blocked for any timeout period.\n");
> -			return PTS_FAIL;
> -		}
> -
> -		if (in_handler != 0) {
> -			perror("Error: signal SIGUSR1 was never received and/or"
> -				" the signal handler was never called.\n");
> +	if (mq_timedsend_errno != EINTR) {
> +		printf("Error: mq_timedsend was NOT interrupted\n");
> +		printf(" signal handler was called %d times\n", in_handler);
> +		printf(" SIGUSR1 signals sent: %d\n", i);
> +		printf(" last mq_timedsend errno: %d %s\n",
> +			mq_timedsend_errno, strerror(mq_timedsend_errno));
> +		if (in_handler == 0) {
> +			printf("Error: SIGUSR1 was never received\n");
>  			return PTS_UNRESOLVED;
>  		}
> +		return PTS_FAIL;
>  	}

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead. 
Download for free and get started troubleshooting in minutes. 
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list