889671 - nsIRequestObserver.onStopRequest is called by nsIAsyncStreamCopier.asyncCopy before data is drained

Status: NEW

(Core :: Networking, defect, P3)

Description

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

I've being running into this issue only on windows for File streams on windows only and for Socket streams on both Linux and Windows. So let me formalize what exactly I'm doing:

So we have nodejs like input/output stream abstractions:
https://github.com/Gozala/addon-sdk/blob/hotfix/io/lib/sdk/io/stream.js

Since I'm running into issues with output streams I'll focus on that one:

`OutputStream` takes instance of `nsIAsyncOutputStream` instance and provides an async API to writing to it. To do that it creates `nsIMultiplexInputStream` into which data is queued. It also creates instance of `nsIAsyncStreamCopier` to asynchronously copy data from multiplexInputStream to provided asyncOutputStream.

Abstraction also emits "drain" and "finish" events. `asyncOutputStream.asyncWait` with `WAIT_CLOSURE_ONLY` flag to know when `asyncOutputStream` is closed. And `nsIAsyncStreamCopier.asyncCopy` is used to dequeue data from multiplexInputStream and and copy a next one. If `multiplexInputStream` is empty `drain` even is emitted. Actual source for this can be found on the following link:
https://github.com/Gozala/addon-sdk/blob/hotfix/io/lib/sdk/io/stream.js#L222-L400


Now problem I run into when dealing with file streams is following:

`OutputStream` is initialized with `nsIAsyncStream` instance created via `nsIStreamTransportService`'s  `createOutputTransport` that is created from
`nsIFileOutputStream` with `OPEN_UNBUFFERED` flag (although that flag does not seems to make any difference).

Issue is `drain` event is called by `onStopRequest` handler is invoked before data actually makes into a file. Closing fileStream then actually prevents data from getting there at all. (This seems only happen on windows).


Now I managed to fix that issue by registering above mentioned observers on the fileOutputStream instead of asyncOutputStream created via transport service. That unfortunately get's into a way of abstracting stream interface in nodejs like API.

Issue that I can not seem to fix is somewhat identical. Same OutputStream is also used to wrap `nsIAsyncOutputStream` created from `nsISocketTransport`'s `openOutputStream`. Issue I'm running into is raises when closing socket after data is drained but same as in file case `onStopRequest` is fired before data is actually drained to underlying fd so closing it then prevents data to ever make it to the other end. In other words last data chunk does not makes it to the peer. This issue occurs on windows and linux but not on osx.

I wonder if I'm doing anything wrong or if this is a bug ? Some workarounds to fix later case would also help.

Comment 1

[Josh Matthews [:jdm]]

Maybe Benjamin's years of experience with XPCOM streams can shed some light.

Comment 2

[Benjamin Smedberg]

Streams aren't me, they are necko code that just happens to live in XPCOM.

Comment 3

[Doug Turner (:dougt)]

jason, do you know what be going on here?

Comment 4

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 5

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258