From: Jan S. <jst...@re...> - 2013-08-12 15:32:29
|
----- Original Message ----- > From: ch...@su... > To: "Jan Stancek" <jst...@re...> > Cc: ltp...@li... > Sent: Monday, 12 August, 2013 4:43:15 PM > Subject: Re: [LTP] [PATCH v3 2/2] open_posix_testsuite/../mq_timedsend/12-1: fix race > > Hi! > > Signed-off-by: Jan Stancek <jst...@re...> > > --- > > 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 :( Back to pipes? :-) > > 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. This sounds promising, I'll look into using barriers for v4. Thanks, Jan > > > +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 > ch...@su... > |