1309681 - NSPR pipes became non-blocking by default

Status: RESOLVED WONTFIX

(NSPR :: NSPR, defect)

Description

[David Woodhouse]

The patch applied to address bug 1289145 had the effect of making pipes non-blocking by default — which was noted in that bug and an alternative fix proposed, but 4.13 shipped with this incompatible change anyway.

It broke Firefox's automatic NTLM single-sign-on under Linux, because it doesn't cope with getting -EAGAIN when reading from a pipe to the ntlm_auth helper process. 

https://bugzilla.redhat.com/show_bug.cgi?id=1383918

Comment 1

[Honza Bambas (:mayhemer)]

I assume the change we've made is this:
https://hg.mozilla.org/projects/nspr/rev/c5f7511ed27673f72c9b4bef15b4e476bc98f402

Dragana, there are two more NSPR patches from Wan-Teh in bug 1289145, that seem (at the first look only!) like an alternative solution potentially fixing both bug 1289145 and this bug.

If we don't want to accept those, could we export an API to set the nonblocking flag on secret and use it only for the pollable event?  (It seems like the patch at [1] is doing exactly that..)

[1] https://bugzilla.mozilla.org/attachment.cgi?id=8790334&action=diff

Comment 2

[Dragana Damjanovic [:dragana]]

(In reply to Honza Bambas (:mayhemer) from comment #1)
> I assume the change we've made is this:
> https://hg.mozilla.org/projects/nspr/rev/
> c5f7511ed27673f72c9b4bef15b4e476bc98f402
> 
> Dragana, there are two more NSPR patches from Wan-Teh in bug 1289145, that
> seem (at the first look only!) like an alternative solution potentially
> fixing both bug 1289145 and this bug.
> 
> If we don't want to accept those, could we export an API to set the
> nonblocking flag on secret and use it only for the pollable event?  (It
> seems like the patch at [1] is doing exactly that..)
> 
> [1] https://bugzilla.mozilla.org/attachment.cgi?id=8790334&action=diff

On solution is to use a socket pair on unix as well, but I prefer not to use it.

The second solution [1] means that we need to use PR_NewPollableEvent, which complicates our PollableEvent  and probably need rewriting a large portion of it.

So I think the solution is to expose _PR_MakePipeNonblock(PRFileDesc *fd) function. In the patch [1] it is an internal function. 

I will make a patch.

Comment 3

[Dragana Damjanovic [:dragana]]

Attachment: bug_1309681.patch

Comment 4

[Kai Engert [:KaiE:]]

We should assume that there are multiple consumers of NSPR, and that Firefox isn't the only one.

It would be best to restore the original behavior of NSPR, which avoids the regression, and the original bug 1289145 should get fixed in a different way.

Comment 5

[Kai Engert [:KaiE:]]

Comment on attachment 8800687 [details] [diff] [review]
bug_1309681.patch

You shouldn't implement PR_MakePipeNonblock twice. Please remove one from one of the .c files.

A better name might be PR_SetPipeNonblocking.

Would it be a better API to allow the caller to specify a parameter, that selects between blocking and nonblocking?

Should optimized builds fail, if the function is called for a non-pipe fd, instead of only debug-build assertion?

Comment 7

[Dragana Damjanovic [:dragana]]

Attachment: bug_1309681.patch

incorporate comment #5.