Re: Problem interrupting threads in PR_Accept() wiht PR_Interrupt()

On Thu, Aug 7, 2008 at 1:17 AM, Joachim Ziegler <[email protected]> wrote:
>
> The original task as given by my project leader was to write a portable
> demo server that does not loose any request (except for the ones caused
> by network timeouts or network problems, which cannot be avoided). That
> is, after a request has been PR_Accept()ed, it must run to full
> completion, even in the case that the time to serve the request is long
> and that during this time the administrator signals the server to go down.
>
> So I formulated the while loop now as follows, including your hint and
> introducing Boolean variables worker_busy[]:
>
>  while( !shutdown_workers ){
>
>    if((fd = PR_Accept( sock, NULL, PR_INTERVAL_NO_TIMEOUT ))
>       == NULL) {
>      if(PR_GetError() == PR_PENDING_INTERRUPT_ERROR)
>        break; /* that's OK: master has signaled an interrupt */
>      else {
>        oops("PR_Accept in thread_start()");
>        continue;
>      }
>    }
>
>    /* RACE HERE */
>
>    worker_busy[nthread] = 1;
>    handle_call(fd, nthread);
>    PR_Close( fd );
>    worker_busy[nthread] = 0;
>  }
>
>
> The master thread first checks that all workers are not busy before
> sending PR_Interrupt(). But I realize that there is a tiny race
> condition in this approach, namely that the interrupt signal occurs
> exactly while the thread is in the line indicated by "RACE HERE". Then
> the request is accepted, but the thread will be interrupted later in
> PR_Send() or PR_TransmitFile(). I cannot combine PR_Accept() and setting
> worker_busy to TRUE into an atomar operation.
>
> Is there any solution to this problem? Can I make PR_Send() and
> PR_TransmitFile() ignore the PR_Interrupt()?

You can use PR_BlockInterrupt and PR_UnblockInterrupt to
block and unblock PR_Interrupt.  I just checked our implementation.
We're not checking the "interrupt blocked" flag in some places,
for example,
http://mxr.mozilla.org/nspr/source/nsprpub/pr/src/pthreads/ptio.c#601

599                 rv = poll(&tmp_pfd, 1, msecs);
600
601                 if (self->state & PT_THREAD_ABORTED)  <=== BUG
602                 {
603                     self->state &= ~PT_THREAD_ABORTED;
604                     op->result.code = -1;
605                     op->syserrno = EINTR;
606                     op->status = pt_continuation_done;
607                     return;
608                 }

We should be using if (_PT_THREAD_INTERRUPTED(self))
on line 601.  If you want to use this approach, we'll need to fix
this bug.  You can search for all such tests in ptio.c and change
them to use the _PT_THREAD_INTERRUPTED macro.

You can also use a solution that Julien or Nelson suggested before:
your master thread can connect to the worker threads and issue
a special shutdown command to shut them down cleanly.  This
solution doesn't use PR_Interrupt.  But your application protocol
must allow this special shutdown command.  For example, suppose
the server is an HTTP server.  You can define a dummy
resource name "GET /shutdown HTTP/1.1" as the special shutdown
command.

Wan-Teh