Ah, it looks like the bug was introduced in 049903e333f8 which refactored
if (ret === false)
state.needDrain = true;
into
state.needDrain = !ret;
which is not actually equivalent :)
On Wed, Nov 13, 2013 at 12:13 AM, David Glasser <[email protected]> wrote:
> To be a little more concrete:
>
> var net = require('net');
> var assert = require('assert');
>
> var port = 8989;
> var server = net.createServer();
> server.listen(port, function () {
> var client = net.connect(port, function () {
> assert(!client.write(Array(16500).join('x') + '\n'));
> if (process.env.BREAKIT)
> assert(client.write('foobar\n'));
> client.on('drain', function () {
> console.log("drained!");
> process.exit(0);
> });
> });
> });
>
>
>
> When you run this with the BREAKIT environment variable set, it hangs
> instead of logging and exiting. That seems wrong to me; does anyone agree?
>
>
>
> On Tue, Nov 12, 2013 at 11:25 PM, David Glasser <[email protected]>wrote:
>
>> The docs for .write() say that "This return value is strictly advisory.
>> You MAY continue to write, even if it returns false."
>>
>> Let's say you call .write() on a Stream.Writable and it returns false.
>> You now expect 'drain' to be emitted at some point. Before the event comes
>> (perhaps even before going back to the event loop), you call .write() on it
>> again, and this time it returns true.
>>
>> Do you expect 'drain' to be emitted?
>>
>> In the current implementation, it's possible that it won't be emitted.
>> Say the first call to write is given data larger than the high water mark,
>> but still small enough that the kernel accepts all of it at once, so that
>> the call to `stream._write` from `doWrite` calls its callback synchronously
>> with `_writableState.sync` true.
>>
>> Then in `onwrite`, `onwriteStateUpdate` will immediately reduce
>> `_writableState.sync` back to 0, but the 'drain' won't be fired until
>> `afterWrite` is called via `process.nextTick`.
>>
>> But another immediate call to `write` with a small amount of data will
>> reset `_writableState.needDrain` to false, and so when `afterWrite` calls
>> `onwriteDrain`, the event won't be fired.
>>
>> Is this intentional or a bug?
>>
>>
>>
>>
>> This is problematic if you take a 0.8-style stream and pipe it (as the
>> source) into a 0.10-style Stream.Writable. `Stream.prototype.write`
>> (defined in stream.js) pauses the source any time it sees `write` return
>> false. But if it's possible for an immediately-following write to inhibit
>> the drain event, the source will never get resumed! If the behavior
>> described above is intentional, does that mean that this is a bug in
>> Stream.prototype.write?
>>
>> --dave
>>
>
>
--
--
Job Board: http://jobs.nodejs.org/
Posting guidelines:
https://github.com/joyent/node/wiki/Mailing-List-Posting-Guidelines
You received this message because you are subscribed to the Google
Groups "nodejs" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/nodejs?hl=en?hl=en
---
You received this message because you are subscribed to the Google Groups
"nodejs" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
