450224 - PR_Interrupt() interrupts PR_TransmitFile() while PR_BlockInterrupt() in effect

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[Joachim Ziegler]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.15) Gecko/20080612 Iceweasel/2.0.0.15 (Debian-2.0.0.15-0etch1)
Build Identifier: 4.7.1

The summary says it. I have 5 worker threads and 1 master thread in a server application. A worker thread, after doing a PR_Accept(), immediately calls PR_BlockInterrupt() so that it can serve the request to full completion. After it has served the request, it calls PR_UnblockInterrupt() so that it can be interrupted by the worker thread issuing a PR_Interrupt() while the worker is blocking in PR_Accept(). 

The bug: The worker's PR_Interrupt() also interrupts the worker while it is in a PR_TransmitFile():

MASTER[11505]: Sending interrupt signal to worker threads.
MASTER[11505]: Thread 0 is blocked. Sent PR_Interrupt.
MASTER[11505]: Thread 1 is blocked. Sent PR_Interrupt.
MASTER[11505]: Thread 2 is blocked. Sent PR_Interrupt.
MASTER[11505]: Thread 3 is blocked. Sent PR_Interrupt.
MASTER[11505]: Thread 4 is blocked. Sent PR_Interrupt.
MASTER[11505]: Joining worker threads.
oops: PR_TransmitFile in do_cat: -5993: Operation interrupted by another thread
        [11505] Thread 2 is shutting down.
oops: PR_TransmitFile in do_cat: -5993: Operation interrupted by another thread
        [11505] Thread 0 is shutting down.
MASTER[11505]: Thread 0 successfully joined.
oops: PR_TransmitFile in do_cat: -5993: Operation interrupted by another thread
        [11505] Thread 3 is shutting down.
oops: PR_TransmitFile in do_cat: -5993: Operation interrupted by another thread
        [11505] Thread 1 is shutting down.
oops: PR_TransmitFile in do_cat: -5993: Operation interrupted by another thread
        [11505] Thread 4 is shutting down.


Reproducible: Always

Comment 1

[Wan-Teh Chang]

Joachim,

Could you add some printf statements to pt_TransmitFile and
the functions it calls to identify where it gets interrupted?
pt_TransmitFile (in ptio.c) has several implementations, so
you need to use the Linux code.

If you don't have time, I'll try to give you a patch later
this week.

Comment 2

[Joachim Ziegler]

Hello Wan-Teh,

it seems that your original instructions to replace

if (self->state & PT_THREAD_ABORTED)
by
if (_PT_THREAD_INTERRUPTED(self))

is enough to solve the problem: I forgot to set my LD_LIBRARY_PATH to point to my own version of the NSPR shared lib, so I never run my self-compiled code, but only the NSPR as originally installed on my Debian Linux box.

I'm sorry for that. 

Now PR_TransmitFile() si not interrupted any more.

Joachim

Comment 3

[Wan-Teh Chang]

Joachim,

No problem.  Glad to hear that using _PT_THREAD_INTERRUPTED(self)
fixes this bug.  Could you attach your patch to this bug report?

Comment 4

[Wan-Teh Chang]

Attachment: Proposed patch

We should test the PT_THREAD_ABORTED flag using the
_PT_THREAD_INTERRUPTED macro, which takes into account
whether interrupts are being blocked.

Comment 5

[Joachim Ziegler]

Attachment: if (_PT_THREAD_INTERRUPTED(self))

Comment 6

[Joachim Ziegler]

I posted the attachment, see above. Unfortunately, I hit RETURN before I could give a detailed explanation.

This patch replaces 4 times the test

if (self->state & PT_THREAD_ABORTED)

by

if (_PT_THREAD_INTERRUPTED(self))

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Attachment: Joachim's modification in patch form

Joachim, Thanks for the sources.
Here is your modified source in the form that bugzilla expects for patches.
It is the output of 
  cvs diff -pu5 ptio.c 

There appears to be a whitespace problem with it, but other than that, 
it fits your description. 

Wan-Teh, would you prefer that I review this new patch of Joachim's ?

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 333691 [details] [diff] [review]
Proposed patch

The two patches are identical except for whitespace and the "modeline" that is the first line of the file. 
Since this patch has no whitespace issues, I'm giving it r+.

Comment 9

[Wan-Teh Chang]

I checked in the patch on the NSPR trunk for NSPR 4.7.2.

Checking in ptio.c;
/cvsroot/mozilla/nsprpub/pr/src/pthreads/ptio.c,v  <--  ptio.c
new revision: 3.109; previous revision: 3.108
done