[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#460512: closed by Aurelien Jarno <aurelien@aurel32.net> (Re: Bug#460512: libc6-dev: pthread_cancel causes sigsegv in receiving thread)



"Also sprach Debian Bug Tracking System:"
> >    signal 11 received in pid 21020 after 1 cancel signals
> >    Segmentation fault
> > 
> > The thread is just sitting in a nanosleep, as I said.
> > 
> > It makes no difference if a handler for SEGV is set or not - it just
> > prints the output message.
> 
> Compiling your program with -Wall shows too much warnings:

Guys .. I bothered to compile and run this on a plane between madrid and
birmingham, and I sent it to you from an airport, before I bothered to
feed and warm _myself_.  I was worn out.  Exercise yourselves before
asking me to do more.

FYI, case you didn't try it, there are NO warnings returned withot Wall,
and WITH the compiler flags entered in the report, which are the
perfectly normal -g -O2 (and a great deal many others, which I tried,
such as -O0, -O1).  If your sense of urgency and obligation is so low
that you won't fix these little extra things that the compiler DOESN'T
complain about by yourself, if it bothers you, then fix your own pthread
lib!  You're complaining about your own head.  Not mine.  So fix your
head (and I understand perfectly that compiler warnings can show up
subtle problems).

I.e.  Fix the problem, not your perceptions of the problem (do that too,
but silently). The problem is that cancel causes a sigfault in the
receiving thread. Send it any way you like. If somebody sends you a
program code to save YOU time investigating it, don't send the code
back. That's like sending back a christmas present because the sender
forgot to tie the ribbon properly.

> test.c:24: warning: type defaults to ?int? in declaration of ?t?

So what? If you want to declare static int t, instead of static t, go
ahead!


> test.c: In function ?print_error?:
> test.c:32: warning: format ?%s? expects type ?char *?, but argument 3
> has type ?int?

So what? It's only a printout statement on errors, and there aren't any
printed during a run.  In my present copy I have

static void
print_error (int err, char *fnname, int line)
{
    error_at_line(0, err, __FILE__, line, " ");
}

and I don't know what you have. Whatever you have, replace it with
something you like.

> test.c: In function ?sighandler?:
> test.c:40: warning: format ?%d? expects type ?int?, but argument 4 has
> type ?long int?

Fantastic .. the only signal that is handled is the one that
shouldn't be received. And long and int are the same size on i386, so go
suck on a boot.

> test.c: In function ?tfn?:
> test.c:94: warning: format ?%d? expects type ?int?, but argument 3 has
> type ?long unsigned int?

Ditto.

> test.c:94: warning: format ?%d? expects type ?int?, but argument 4 has
> type ?long int?

Ditto.

> test.c:80: warning: unused variable ?err?

Good.

> test.c:103: warning: control reaches end of non-void function

Fix it if you care. Add a return NULL, or 0, or whatever.

> Most notably strerror_r returns 0 when it succeeds and -1 when it fails,
> so printing it with %s may lead to segfault.

No, your comment is nonsense: this only prints errors if they occur, and
there aren't any that occur.  But who cares anyway.  Don't use strerror
if it bothers you (and the only real problem is that strerror_r prints
nonsense in a thread context anyway).



> Please fix those warnings, and if the bug is still present, come with a
> warning-clean testcase.

You fix the "problems".  All you have to do is send a cancel signal to a
thread.  I _bothered_ to do it for you.

Peter

And iff you can't do it. Try the below. I used sleep this time, not
nanosleep. Put nanosleep back if you like nanosleep because it doesn't
use any signals at all.

  betty:/home/oboe/ptb% gcc -Wall -g -O2 -pthread -o testp2 testp2.c
  betty:/home/oboe/ptb% ./testp2
  (9722) parent waits on mutex
  (9722) parent obtains mutex
  (9722) parent waits on signal (mutex released)
  (9723) thread waits on mutex
  (9723) thread obtains mutex
  (9722) created thread 9723
  (9723) thread signals parent
  (9723) thread unlocks mutex
  (9722) parent receives signal (mutex gained)
  (9722) parent releases mutex
  (9722) parent sends cancel 0 to thread
  signal 11 received in pid 9723 after 1 cancel signals
  Segmentation fault


/* compile with
 * gcc -Wall -g -O2 -pthread -o testp2 testp2.c
 */


#include <stdio.h>
#include <stdlib.h>
#include <error.h>
#include <errno.h>
#include <time.h>
#include <signal.h>
#include <sys/types.h>
#define _GNU_SOURCE             /* or _BSD_SOURCE or _SVID_SOURCE */
#include <unistd.h>
#include <sys/syscall.h>

#define __USE_GNU 1
#include <pthread.h>

static pthread_mutex_t mutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;

static int kcount;
static int debug = 1;

static int t;                       // set when thread is ready to play

static void
print_error (int err, char *fnname, int line)
{
    error_at_line(0, err, __FILE__, line, " ");
}

static void
sighandler (int k)
{
    fprintf (stderr,
             "signal %d received in pid %ld after %d cancel signals\n", k,
             syscall (SYS_gettid), kcount);
    debug++;
    signal (k, SIG_IGN);
}

#define DEBUG(s ...) if (debug) { \
     fprintf(stderr, "(%ld) ", syscall(SYS_gettid)); \
     fprintf(stderr, s); \
  }

static void
do_thread_stuff (void)
{

    // just wait for 2s in sleep or nanosleep
    int err;
    int tts = 2;

    // this would save the day, but it's silly. Default is ENABLE.
    //pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL);

  complete_nanosleep:

    // add this for no effect for extra fun
    //pthread_testcancel();

    err = sleep(tts);
    //err = nanosleep (&req, &rem);

    if (err) {
        switch (errno) {

          // for using nanosleep instead
          //case EINVAL:
          //    print_error (errno, "nanosleep", __LINE__);
          //    break;
          //case EFAULT:
          //    print_error (errno, "nanosleep", __LINE__);
          //    break;
          //case EINTR:
          //    req = rem;
          //    goto complete_nanosleep;
          //    break;

              tts = err;
              goto complete_nanosleep;
              break;
        }
    }
    pthread_testcancel();
}

static void *
tfn (void *tdata)
{

    //int err;

    // can try ASYNC mode here - no difference
    //err = pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
    //if (err) {
    //    print_error(err, "pthread_setcanceltype", __LINE__);
    //    return NULL;
    //}

    /* 
     *   - tell the parent waiter that we are ready to play
     */

    DEBUG ("thread waits on mutex\n");
    pthread_mutex_lock (&mutex);
    DEBUG ("thread obtains mutex\n");
    fprintf (stderr, "(%ld) created thread %ld\n", (long)(pthread_t) tdata,
             syscall (SYS_gettid));
    t++;
    DEBUG ("thread signals parent\n");
    pthread_cond_signal (&cond);

    DEBUG ("thread unlocks mutex\n");
    pthread_mutex_unlock (&mutex);

    do_thread_stuff ();         // just wait in nanosleep
    return NULL;

}

int
main ()
{

    pthread_t tid;
    int err;

    signal (SIGSEGV, sighandler);

    DEBUG ("parent waits on mutex\n");
    pthread_mutex_lock (&mutex);
    DEBUG ("parent obtains mutex\n");

    err =
     pthread_create (&tid, NULL, tfn, (void *) (long) (syscall (SYS_gettid)));
    if (err) {
        print_error (err, "pthread_create", __LINE__);
        return -err;
    }

    while (t <= 0) {
        DEBUG ("parent waits on signal (mutex released)\n");
        pthread_cond_wait (&cond, &mutex);
        DEBUG ("parent receives signal (mutex gained)\n");
    }

    // detach or not makes no difference
    //err = pthread_detach (tid);
    //if (err) {
    //    print_error (err, "pthread_detach", __LINE__);
    //    return -err;
    //}

    /* the thread is now in async mode and we have the lock on the
     * mutex. No other lock is held or will ever be held. 
     * Release the lock for symmetry and go play with cancel.
     */

    DEBUG ("parent releases mutex\n");
    pthread_mutex_unlock (&mutex);

    while (1) {

        // cancel the thread every millisecond
        struct timespec req = { 0, 1000000, }, rem;

      complete_nanosleep:
        err = nanosleep (&req, &rem);
        if (err) {
            switch (errno) {
              case EFAULT:
                  print_error (errno, "nanosleep", __LINE__);
                  break;
              case EINTR:
                  req = rem;
                  goto complete_nanosleep;
                  break;
              case EINVAL:
                  print_error (errno, "nanosleep", __LINE__);
                  break;
            }
        }

        DEBUG ("parent sends cancel %d to thread\n", kcount);
        err = pthread_cancel (tid);

        if (err) {
            print_error (err, "pthread_cancel", __LINE__);
            return err;
        }
        kcount++;

    }
    // not reached
}






Reply to: