1384633 - Expose OVERLAPPED or CancelIO and GetOverlappedResult as a workaround in NSPR 4.17, only

Status: RESOLVED FIXED

(NSPR :: NSPR, enhancement)

Description

[Dragana Damjanovic [:dragana]]

To be able to close a socket that has been started with TFO but still has not finished connecting we will need to:
1) cancel the overlapped io (ConnectEx used by TFO uses an overlapped io) using CancelIo
2) wait until the overlapped io is finished, i.e. properly canceled. As soon as  GetOverlappedResult returns true or error that is not ERROR_IO_PANDING the overlapped io has been finished.

There is to ways to implement that:

1) PR_FileDesc2OVERLAPPED function that will return pointer to the overlapped structure. This is easier solution.
2) implement functions PR_CancelIo and PR_GetOverlappedResult similar to all other functions that we have in PRIOMethods. This will need more work, we will need to use 2 of PRReservedFN slots and this will be windows specific. If we go this way it is a question should we also expose ConnectEx. Currently it is imitating linux api and it does not expose the overlapped structure, I would suggest that it should and we should let the app (in this case platform) deal with the overlapped structure.

Comment 1

[Dragana Damjanovic [:dragana]]

Attachment: bug_expose_overlapped_struct_v1.patch

This is a patch for solution 1). The patch is almost ready, but I am submitting it now to start a discussion.

Comment 2

[Dragana Damjanovic [:dragana]]

I would like to hear your opinion, Kai, Patrick, Honza?

Comment 3

[Dragana Damjanovic [:dragana]]

Attachment: bug_expose_overlapped_struct_v1.patch

Comment 4

[Dragana Damjanovic [:dragana]]

If we agree on this option, you can also review the patch.

Comment 5

[Honza Bambas (:mayhemer)]

Comment on attachment 8890895 [details] [diff] [review]
bug_expose_overlapped_struct_v1.patch

Review of attachment 8890895 [details] [diff] [review]:
-----------------------------------------------------------------

Not sure what you need this for, tho.

After you do CancelIo (there is no guarantee it will actually do something :)) you need to wait for the op completion prior to closing the socket (=destroying PRFileDescr and the ol address).  the overlapped struct can keep an event that is notified when the op is done.  you may need a separate windows only small native thread to monitor it, probably.  An example in our code: https://dxr.mozilla.org/mozilla-central/rev/7d2e89fb92331d7e4296391213c1e63db628e046/netwerk/socket/nsNamedPipeIOLayer.cpp#428

::: nsprpub/pr/src/io/prsocket.c
@@ +1668,5 @@
>  
> +#if defined(_WIN64) && defined(WIN95)
> +/* Expose OVERLAPPED if present. */
> +PR_IMPLEMENT(void)
> +PR_FileDesc2OVERLAPPED(PRFileDesc *fd, void *ol)

void **ol?

or rather return void* as a result?

Comment 6

[Dragana Damjanovic [:dragana]]

(In reply to Honza Bambas (:mayhemer) from comment #5)
> Comment on attachment 8890895 [details] [diff] [review]
> bug_expose_overlapped_struct_v1.patch
> 
> Review of attachment 8890895 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Not sure what you need this for, tho.
> 
> After you do CancelIo (there is no guarantee it will actually do something
> :)) you need to wait for the op completion prior to closing the socket
> (=destroying PRFileDescr and the ol address).  the overlapped struct can
> keep an event that is notified when the op is done.  you may need a separate
> windows only small native thread to monitor it, probably.

This is why I ask for need info. This version does not ask for changing already existing TCPSendTo.
I have a patch for the necko code that makes use of this. I will attache it to bug 1363372.

If we want to use events I will need to do version 2 form the description.

Comment 7

[Patrick McManus [:mcmanus]]

having option 1, is not a bad thing in and of itself. Plausibly we could go with that and then also add option 2 going forward. (this might also let us get field data on whether this is at the core of the TFO issue without getting committed to an API).

however, if a windows app can't successfully use the new hooks we put in NSPR to support TFO without know what magic to do on the overlapped it gets from option 1 - then I think the nspr hooks need to extended in some way (maybe option 2 - maybe something that feels more cross platform). I would not block option 1 and some testing on that though.

Comment 8

[Honza Bambas (:mayhemer)]

(In reply to Patrick McManus [:mcmanus] from comment #7)
> having option 1, is not a bad thing in and of itself. 

The problem is that CancelIo may not cancel synchronously, so that you still can't close the socket.

Comment 9

[Kai Engert [:KaiE:]]

I cannot comment on the purpose of this API, but some general comments:

The .def file will need to specify the correct NSPR version number in which the new API gets added, so you'll have to add a new section at the end of the file which contains the new API.

Your API seems to be Windows-only. I don't know if NSPR already has APIs that are available only on a subset of the platforms. If not, I'd suggest that the function is made available on all platforms, but on the platforms where you don't provide an implementation, that the API returns an error.

Comment 10

[Dragana Damjanovic [:dragana]]

(In reply to Honza Bambas (:mayhemer) from comment #8)
> (In reply to Patrick McManus [:mcmanus] from comment #7)
> > having option 1, is not a bad thing in and of itself. 
> 
> The problem is that CancelIo may not cancel synchronously, so that you still
> can't close the socket.

I know. Therefore, if you look at patch in bug 1363372 I put all this fd-s into an array and check GetOverlappedResult every poll cycle. I do not need to wake up poll more often and I do not expect that there will be many fd-s in the array. There is a note that we may leak fd-s on shoutdown (I could fix that by adding a loop for let's say 500ms)

Comment 11

[Dragana Damjanovic [:dragana]]

(In reply to Kai Engert (:kaie:) from comment #9)
> I cannot comment on the purpose of this API, but some general comments:
> 
> The .def file will need to specify the correct NSPR version number in which
> the new API gets added, so you'll have to add a new section at the end of
> the file which contains the new API.
> 

I know that I was not sure which one is the next one. I wil fix that before landing

> Your API seems to be Windows-only. I don't know if NSPR already has APIs
> that are available only on a subset of the platforms. If not, I'd suggest
> that the function is made available on all platforms, but on the platforms
> where you don't provide an implementation, that the API returns an error.

I made the api for all platforms t first, but I notice that there are some that are only nt (nspr has 2 windows version nt and w95. w95 is the newer one and it is used by firefox). I will double check this.

Comment 12

[Dragana Damjanovic [:dragana]]

(In reply to Patrick McManus [:mcmanus] from comment #7)
> having option 1, is not a bad thing in and of itself. Plausibly we could go
> with that and then also add option 2 going forward. (this might also let us
> get field data on whether this is at the core of the TFO issue without
> getting committed to an API).

This is the reason why I suggested option 1. I wanted to get some field data until mid-next week if possible (therefore I have spend some time trying to figure out if option 1 is possible)

> 
> however, if a windows app can't successfully use the new hooks we put in
> NSPR to support TFO without know what magic to do on the overlapped it gets
> from option 1 - then I think the nspr hooks need to extended in some way
> (maybe option 2 - maybe something that feels more cross platform). I would
> not block option 1 and some testing on that though.


About option 2: we will need a separate thread to wait for completion events. We have bad experience about windows kernel and multiple thread. In this case it will only be the OVERLAPPED structure which is probably fine.

Comment 13

[Honza Bambas (:mayhemer)]

(In reply to Dragana Damjanovic [:dragana] from comment #10)
> (In reply to Honza Bambas (:mayhemer) from comment #8)
> > (In reply to Patrick McManus [:mcmanus] from comment #7)
> > > having option 1, is not a bad thing in and of itself. 
> > 
> > The problem is that CancelIo may not cancel synchronously, so that you still
> > can't close the socket.
> 
> I know. Therefore, if you look at patch in bug 1363372 I put all this fd-s
> into an array and check GetOverlappedResult every poll cycle. I do not need
> to wake up poll more often and I do not expect that there will be many fd-s
> in the array. There is a note that we may leak fd-s on shoutdown (I could
> fix that by adding a loop for let's say 500ms)

I had a similar idea.  Go for it!

Comment 14

[Dragana Damjanovic [:dragana]]

I have run a version of this patch and patch from bug 1363372 for a couple of hours. I have changed the code a bit so that TFO connection is made for each new connection and immediately canceled after a 1ms. I have seen around 100 calls to CancelIO and GetOverlappedResponse returns success os the error ERROR_OPERATION_ABORTED. I have not seen GetOverlappedResponse returning ERROR_IO_PENDING.

I will upload an updated patch. I have made function available for all platforms.(I looked and I think we do not have any expose function that is platform specific. It was my mistake previously.)

Comment 15

[Dragana Damjanovic [:dragana]]

Attachment: bug_expose_overlapped_struct_v1.patch

Comment 16

[Patrick McManus [:mcmanus]]

Comment on attachment 8891355 [details] [diff] [review]
bug_expose_overlapped_struct_v1.patch

Review of attachment 8891355 [details] [diff] [review]:
-----------------------------------------------------------------

this is technically in private, so I think we can certainly live with it at least for the period until we're confident it leads to a solution around TFO.

How are you going to deal with the feature-detection and link time issues? (i.e. a nspr lib without this against a gecko with it?) I know in practice this isn't an issue, as we bundle nspr on windows, but do you need some kind of runtime symbol lookup?

I'll have honza do the r? as he's more comfortable with windows specific code.

::: nsprpub/pr/src/io/prsocket.c
@@ +1678,5 @@
> +    if (!fd) {
> +        PR_SetError(PR_INVALID_ARGUMENT_ERROR, 0);
> +        return PR_FAILURE;
> +    }
> +    

whitespace nit

::: nsprpub/pr/src/nspr.def
@@ +459,5 @@
>  ;+# but we neglected to add it to nspr.def until NSPR 4.12
>  ;+NSPR_4.12 {
>  ;+      global:
>  		PR_DuplicateEnvironment;
> +    PR_FileDesc2OVERLAPPED;

whitespace nit

Comment 17

[Dragana Damjanovic [:dragana]]

(In reply to Patrick McManus [:mcmanus] from comment #16)
> Comment on attachment 8891355 [details] [diff] [review]
> bug_expose_overlapped_struct_v1.patch
> 
> Review of attachment 8891355 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> this is technically in private, so I think we can certainly live with it at
> least for the period until we're confident it leads to a solution around TFO.
> 
> How are you going to deal with the feature-detection and link time issues?
> (i.e. a nspr lib without this against a gecko with it?) I know in practice
> this isn't an issue, as we bundle nspr on windows, but do you need some kind
> of runtime symbol lookup?
> 

you are right. :)

Comment 18

[Dragana Damjanovic [:dragana]]

Attachment: bug_expose_overlapped_struct_v1.patch

I forgot to change the version.

Comment 19

[Dragana Damjanovic [:dragana]]

I have just seen that 4.16 is beta. So I will need to change the version. Kai, let me know which one is the correct one.

Comment 20

[Kai Engert [:KaiE:]]

Comment on attachment 8891367 [details] [diff] [review]
bug_expose_overlapped_struct_v1.patch

>+#if defined(_WIN64) && defined(WIN95)
>+    *ol = NULL;
>+    if (fd) {
>+        fd = PR_GetIdentitiesLayer(fd, PR_NSPR_IO_LAYER);
>+    }
>+    if (!fd) {
>+        PR_SetError(PR_INVALID_ARGUMENT_ERROR, 0);
>+        return PR_FAILURE;
>+    }
>+
>+    if (!fd->secret->overlappedActive) {
>+        PR_SetError(PR_INVALID_ARGUMENT_ERROR, 0);
>+        return PR_FAILURE;
>+    }
>+
>+    *ol = &fd->secret->ol;
>+    return PR_SUCCESS;
>+#else

Please add:
      PR_SetError(PR_NOT_IMPLEMENTED_ERROR, 0);

>+    return PR_FAILURE;
>+#endif
>+}


We've already released NSPR 4.16 (this week).

Please change to NSPR_4.17 /NSPR_4.16


>+;+NSPR_4.15 {
>+;+      global:
>+		PR_FileDesc2OVERLAPPED;
>+;+} NSPR_4.14;

Comment 21

[Dragana Damjanovic [:dragana]]

Kai, do you know how to check nspr version at runtime? PR_GetVersion?

Comment 22

[Kai Engert [:KaiE:]]

yes

/*
 * Returns a const string of the NSPR library version.
 */
NSPR_API(const char*) PR_GetVersion(void);

Comment 23

[Dragana Damjanovic [:dragana]]

Thanks, Kai.

Comment 24

[Dragana Damjanovic [:dragana]]

Attachment: bug_expose_overlapped_struct_v1.patch

addressed Kai's comments.

Comment 25

[Kai Engert [:KaiE:]]

Thanks a lot for these changes.

I have some additional comments, sorry for being late, I hope they make sense.

- please use a function name that is more descriptive/universal and avoids CAPS.
  Once we add a new API, we must never remove it, so let's select a good name.

  Because the return data type is a universal void*, this API could potentially get implemented on other platforms,
  for a similar purpose, if needed.
  How about this name? => PR_GetPlatformOverlappedIOHandle

  (Maybe you/Honza/Patrick can think of a better name in this style, if my suggestion doesn't seem right.)


- I'm surprised that you have two implementations for your function in two different source files.
  Could that be changed to have only place with 
    PR_IMPLEMENT(PRStatus) PR_FileDesc2OVERLAPPED ?


- The comment explaining the function seems to be more appropriate for the header file.
  
  Could you take inspiration from one of the existing header files, and create a slightly more verbose
  comment that describes the new API?

  E.g. what about ownership of the returned handle, does the caller need to do anything special
  after obtaining it, like destroying?

Comment 26

[Dragana Damjanovic [:dragana]]

(In reply to Kai Engert (:kaie:) (on vacation) from comment #25)
> Thanks a lot for these changes.
> 
> I have some additional comments, sorry for being late, I hope they make
> sense.
> 
> - please use a function name that is more descriptive/universal and avoids
> CAPS.
>   Once we add a new API, we must never remove it, so let's select a good
> name.
> 
>   Because the return data type is a universal void*, this API could
> potentially get implemented on other platforms,
>   for a similar purpose, if needed.
>   How about this name? => PR_GetPlatformOverlappedIOHandle
> 
>   (Maybe you/Honza/Patrick can think of a better name in this style, if my
> suggestion doesn't seem right.)
> 
I will change the name.

> 
> - I'm surprised that you have two implementations for your function in two
> different source files.
>   Could that be changed to have only place with 
>     PR_IMPLEMENT(PRStatus) PR_FileDesc2OVERLAPPED ?
> 

I copied what PR_FileDesc2NativeHandle. Linux uses pthread and windows prsocket file.

Comment 27

[Dragana Damjanovic [:dragana]]

Attachment: bug_expose_overlapped_struct_v1.patch

Comment 28

[Honza Bambas (:mayhemer)]

Comment on attachment 8891419 [details] [diff] [review]
bug_expose_overlapped_struct_v1.patch

Review of attachment 8891419 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm

Comment 29

[Dragana Damjanovic [:dragana]]

Comment on attachment 8891419 [details] [diff] [review]
bug_expose_overlapped_struct_v1.patch

Kai, is on PTO. and he wrote me in an e-mail that I could land this.

I have updated the patch according to his last comments.

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/842f255d51dc
UPGRADE_NSPR_RELEASE This is a temporary change to the nspr code in mozilla-central. We believe we have identified a fix to TCP Fast open feature crashes. We would like to verify the fix before making changes in nspr and investing time in doing a proper fix in nspr. The issue has been discussed over e-mails and this push is justifiable before commiitting changes to nspr. Overwriting this push by a new nspr release will not break mozilla-central it will only disable the TCP fast open feature. Patch has been reviewed in bug 1384633. r=mcmanus r=:kaie: r=mayhemer a=jduell a=mcmanus

Comment 31

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/842f255d51dc

Comment 32

[Dragana Damjanovic [:dragana]]

Attachment: bug_cancelIO_nspr_v1.patch

This is a patch that does the same thing as the code that is in necko.

Explanation:
When a socket that have an overlapped IO pending is closed (we only use overlapped IO for TCP Fast Open, that is the only way to implement TCP Fast Open) all pending IOs will be canceled. The OVERLAPPED structures for this IOs needs to be kept alive until the IOs are cancel for real. This can take some time. Out current implementation frees PRFileDesc during PR_Close and that will free the attached OVERLAPPED structure.
This patch calls CancelIO on the socket before closing it and only closes the socket when GetOveralappedResult returns without error WAS_IO_INCOMPLETE.
If the pending IOs are not canceled immediately the PRFileDesc are added to a list. We check this list during each call to PR_Poll.
I did not use "waiting for IO event" function because it will need a separate thread and I wanted to avoid using multiple threads. We had problems using sockets on multiple threads before.

Comment 33

[Dragana Damjanovic [:dragana]]

And I forgot to add, all crashes that we had with TFO are gone.

Comment 34

[Dragana Damjanovic [:dragana]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=242933fa54fda84c3d6facd34637071eb9d517f6

Comment 35

[Honza Bambas (:mayhemer)]

(In reply to Dragana Damjanovic [:dragana] from comment #33)
> And I forgot to add, all crashes that we had with TFO are gone.

\   /
 \O/

Comment 36

[Honza Bambas (:mayhemer)]

Comment on attachment 8898201 [details] [diff] [review]
bug_cancelIO_nspr_v1.patch

Review of attachment 8898201 [details] [diff] [review]:
-----------------------------------------------------------------

You will need to fix the indention issues.

::: nsprpub/pr/include/private/primpl.h
@@ +2170,5 @@
> +  PRFileDesc *fd;
> +  struct _PRFileDescList *next;
> +} PRFileDescList;
> +
> +static PRFileDescList *_fd_waiting_for_overlapped_done = NULL;

some threading assertions around access to this global would be wise

::: nsprpub/pr/src/io/prio.c
@@ +181,5 @@
> +      if (previous) {
> +        previous->next = cur->next;
> +      } else {
> +        _fd_waiting_for_overlapped_done = cur->next;
> +      }

maybe you can PRFileDescList *previous = _fd_waiting_for_overlapped_done; and remove this if branching (just assign to previous->next simply).

@@ +199,5 @@
>  */
>  PR_IMPLEMENT(PRInt32) PR_Poll(PRPollDesc *pds, PRIntn npds, PRIntervalTime timeout)
>  {
> +#if defined(_WIN64) && defined(WIN95)
> +  // For each iteration check if overlapped io are down.

"TFO overlapped io" ?

::: nsprpub/pr/src/io/prsocket.c
@@ +769,5 @@
> +        _fd_waiting_for_overlapped_done = forWaiting;
> +      } else {
> +        while (cur->next) cur = cur->next;
> +        cur->next = forWaiting;
> +      }

for performance sake, should we rather append to the head instead of finding the tail?

Comment 37

[Dragana Damjanovic [:dragana]]

Attachment: bug_cancelIO_nspr_v1.patch

I have added a lock, Firefox does not use _fd_waiting_for_overlapped_done on different threads, but some other application may.

Comment 38

[Dragana Damjanovic [:dragana]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=52c414a3df3e672b1fed93f8f38d31533a81fb6c

Comment 39

[Honza Bambas (:mayhemer)]

Comment on attachment 8899428 [details] [diff] [review]
bug_cancelIO_nspr_v1.patch

Review of attachment 8899428 [details] [diff] [review]:
-----------------------------------------------------------------

Hmm.. I'm afraid that CancelIO and GetOverlappedResult could be blocking.  Not sure how much work that would be, but I'd rather swap the list to a local var (and clear it) under the lock in CheckOverlappedPendingSocketsAreDone() and then process the list locally.  All what's left (still in progress) would then be just readded to the list (again under the lock) preserving of course anything that might have been added during the period the lock was open.

up to you, but I would prefer that.

::: nsprpub/pr/src/md/windows/w95sock.c
@@ +381,5 @@
>  PRInt32
>  _PR_MD_TCPSENDTO(PRFileDesc *fd, const void *buf, PRInt32 amount, PRIntn flags,
>                   const PRNetAddr *addr, PRUint32 addrlen, PRIntervalTime timeout)
>  {
> +    if (!_fd_waiting_for_overlapped_done_lock) {

hmm.. if the init happens on the main thread and this happens very soon on the socket thread there could potentially be a race (unflushed uncore cache) and this could still be null.  but I really don't know how to avoid it...

::: nsprpub/pr/src/md/windows/w95thred.c
@@ +36,5 @@
>      _pr_currentCPUIndex = TlsAlloc();
>  #endif
> +
> +#if defined(_WIN64) && defined(WIN95)
> +    _fd_waiting_for_overlapped_done_lock = PR_NewLock();

where is this released?

Comment 40

[Dragana Damjanovic [:dragana]]

(In reply to Honza Bambas (:mayhemer) from comment #39)
> Comment on attachment 8899428 [details] [diff] [review]
> bug_cancelIO_nspr_v1.patch
> 
> Review of attachment 8899428 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hmm.. I'm afraid that CancelIO and GetOverlappedResult could be blocking. 
> Not sure how much work that would be, but I'd rather swap the list to a
> local var (and clear it) under the lock in
> CheckOverlappedPendingSocketsAreDone() and then process the list locally. 
> All what's left (still in progress) would then be just readded to the list
> (again under the lock) preserving of course anything that might have been
> added during the period the lock was open.
> 
> up to you, but I would prefer that.
> 

Firefox calls these 2 on the same thread so there is no much benefit. Antivirus software was a problem for accessing sockets on different threads.

> ::: nsprpub/pr/src/md/windows/w95sock.c
> @@ +381,5 @@
> >  PRInt32
> >  _PR_MD_TCPSENDTO(PRFileDesc *fd, const void *buf, PRInt32 amount, PRIntn flags,
> >                   const PRNetAddr *addr, PRUint32 addrlen, PRIntervalTime timeout)
> >  {
> > +    if (!_fd_waiting_for_overlapped_done_lock) {
> 
> hmm.. if the init happens on the main thread and this happens very soon on
> the socket thread there could potentially be a race (unflushed uncore cache)
> and this could still be null.  but I really don't know how to avoid it...
> 

We cannot do much here. TFO will be turned off :(

> ::: nsprpub/pr/src/md/windows/w95thred.c
> @@ +36,5 @@
> >      _pr_currentCPUIndex = TlsAlloc();
> >  #endif
> > +
> > +#if defined(_WIN64) && defined(WIN95)
> > +    _fd_waiting_for_overlapped_done_lock = PR_NewLock();
> 
> where is this released?

I have added a release in the updated patch.

Comment 41

[Dragana Damjanovic [:dragana]]

Attachment: bug_cancelIO_nspr_v1.patch

Comment 42

[Dragana Damjanovic [:dragana]]

It can happen that we leak sockets that are in _fd_waiting_for_overlapped_done. But for firefox we cannot afford to wait for all of them to be done. That is one of the reason why I wanted to expose the Overlapped structure and then firefox can do what is the best for Firefox.

Comment 43

[Dragana Damjanovic [:dragana]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=323a03b2ccad4020130b856a25510dc897dec497

Comment 44

[Mike Hommey [:glandium]]

I don't think we should continue adding things like this to nspr. Instead, we should add code to handle all this on the gecko side.

Comment 45

[Dragana Damjanovic [:dragana]]

(In reply to Mike Hommey [:glandium] from comment #44)
> I don't think we should continue adding things like this to nspr. Instead,
> we should add code to handle all this on the gecko side.

What do you meet? the first or the second patch, exposing the OVERLAPPED structure or implement TFO in nspr?

Comment 46

[Mike Hommey [:glandium]]

All of the above, except if you need it for NSS.

Comment 47

[Dragana Damjanovic [:dragana]]

(In reply to Mike Hommey [:glandium] from comment #46)
> All of the above, except if you need it for NSS.

Currently nss does not use TFO.
:ekr prefers to have it completely implemented in nspr (please correct me if I am wrong).

Comment 48

[Dragana Damjanovic [:dragana]]

Attachment: bug_expose_overlapped_struct_v2.patch

Changing the function name.

Comment 49

[Kai Engert [:KaiE:]]

Comment on attachment 8901168 [details] [diff] [review]
bug_expose_overlapped_struct_v2.patch

I'm accepting this temporary patch under the following agreement:

- the temporary function will be removed in NSPR 4.18

- the function will be accessed from Firefox code ONLY on the Windows platform

- Only Firefox 57 on Windows will use that temporary function

- On non-Windows platforms, Firefox 57 NOT have a link time dependency,
  which means, for example, 
  using a Firefox 57 Linux binary combined with a newer NSPR 4.18 will work

- Firefox 58 will no longer use the API

Comment 50

[Kai Engert [:KaiE:]]

Comment on attachment 8899838 [details] [diff] [review]
bug_cancelIO_nspr_v1.patch

Do you still need this patch? I believe you said you don't. If you don't require it, could you please mark it as obsolete?

Comment 51

[Dragana Damjanovic [:dragana]]

(In reply to Kai Engert (:kaie:) from comment #50)
> Comment on attachment 8899838 [details] [diff] [review]
> bug_cancelIO_nspr_v1.patch
> 
> Do you still need this patch? I believe you said you don't. If you don't
> require it, could you please mark it as obsolete?

That is the follow up fix for the issue described here. I will open a separate bug for it and move the patch there.

Comment 52

[Kai Engert [:KaiE:]]

Comment on attachment 8901168 [details] [diff] [review]
bug_expose_overlapped_struct_v2.patch

r=kaie, accepted temporarily for 4.17
Will be removed in 4.18.

Please file a bug to track the removal in 4.18

Comment 53

[Kai Engert [:KaiE:]]

https://hg.mozilla.org/projects/nspr/rev/6a0af7609427