72965 - PR_Poll can time out even when data is available

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[Simon Fraser [no longer active]]

We have a race condition in the PR_Poll code now which can cause it to behave as 
if no data is available on the socket, even when there is data. This causes 
stalls in downloads for HTTP (see bug 60509) which are as long as the poll 
timeout (35 seconds in Necko).

I've established using instrumentation that the problem occurs when the Notifier 
fires before we've put the polling thread to sleep (a classic threaded I/O race 
condition).

Comment 1

[Simon Fraser [no longer active]]

There's another problem here as well, which is the way that we blindly wake the 
polling thread from the Notifier routine (since this thread was set on the fd the 
first time we polled that fd, and was left set). This has the effect that a 
notification that comes in when the PR_Poll thread is not asleep write to wake a 
thread on which io_pending is false, which seems wrong. Maybe PR_Poll should set 
then clear the md.poll.thread members of each fd being polled each time around, 
with the appropriate locks.

Comment 2

[Simon Fraser [no longer active]]

Attachment: _MD_Poll changes to fix this

Comment 3

[Simon Fraser [no longer active]]

These changes fix PR_Poll on Mac thusly:
1. Factor out checking the fds into a new function CheckPollDescs()
2. Factor out setting/clearing the polling thread on those fds into 
SetDescPollThread()
3. Be more careful about where we set the polling thread on the fds, ensuring 
   that we turn off interrupts and hold a lock around the code that sets up the
   polling thread and checks for data on the fds. This fixes the race condition
   that causes this bug.
4. We now clear the polling thread on the fds when coming out of PR_Poll, so that
   the notifier doesn't attempt to wake the wrong thread when called when
   we're not polling.
5. Implement a 0-timeout version that behaves like select().

Note that the macthr.c change is also in the patch in bug 60509.

Taking this bug for myself.

Reviews please.

Comment 4

[larryh (gone)]

r=[e-mail]
To the best of my limited Mac ability, looks OK to me.

Comment 5

[Simon Fraser [no longer active]]

I checked this in to the NSPR tip and the client branch.