Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplex.allowHalfOpen not behaving according to the docs? #4044

Closed
mrkishi opened this issue Nov 26, 2015 · 4 comments
Closed

Duplex.allowHalfOpen not behaving according to the docs? #4044

mrkishi opened this issue Nov 26, 2015 · 4 comments
Labels
stream Issues and PRs related to the stream subsystem.

Comments

@mrkishi
Copy link

mrkishi commented Nov 26, 2015

It seems like Duplex's allowHalfOpen option is not following the documentation behavior:

If set to false, then the stream will automatically end the readable side when the writable side ends and vice versa.

'use strict';

var stream = require('stream');

class RangeSource extends stream.Readable {

    constructor(min, max, options) {

        super(options);

        this.min  = min|0;
        this.max  = max|0;
        this.next = this.min;

    }

    _read() {

        for (let data = this.next; data <= this.max; ++data) {
            if (!this.push(data.toString())) {
                this.next = data + 1;
                return;
            }
        }

        this.push(null);

    }

}

class Sink extends stream.Writable {
    _write(data, encoding, next) { next() }
}

class PushDuplex extends stream.Duplex {

    constructor(options) {

        super(options);

        // uncomment for the correct (?) behavior:
        /*

        this.on('finish', () => {
            if (!this.allowHalfOpen) {
                this.push(null);
            }
        });

        //*/

    }

    _read() {}

    _write(data, enc, next) {
        this.push(data);
        next();
    }

}

var source   = new RangeSource(1, 3);
var allow    = new PushDuplex({ allowHalfOpen: true });
var disallow = new PushDuplex({ allowHalfOpen: false });
var sink     = new Sink();

source
    .on('data', (data) => console.log('source      data:', data.toString()))
    .on('end',  ()     => console.log('source      ended'));

source.pipe(allow, { end: true })
    .on('data', (data) => console.log('allow       data:', data.toString()))
    .on('end',  ()     => console.log('allow       ended'))
    .pipe(sink);

source.pipe(disallow, { end: true })
    .on('data', (data) => console.log('disallow    data:', data.toString()))
    .on('end',  ()     => console.log('disallow    ended'))
    .pipe(sink);

You can see the readable side isn't closed when the writable side ends. Are the docs, Duplex or am I wrong?

@mscdex mscdex added the stream Issues and PRs related to the stream subsystem. label Nov 26, 2015
@mscdex
Copy link
Contributor

mscdex commented Nov 26, 2015

/cc @nodejs/streams

@mrkishi
Copy link
Author

mrkishi commented Mar 20, 2016

I'm still seeing this behavior on v5.9.0. Did I misunderstand the docs?

@calvinmetcalf
Copy link
Contributor

from what I can tell looking at the code the docs are wrong in that it only closes the writable side if the readable side ends.

Trott added a commit to Trott/io.js that referenced this issue Jul 7, 2017
If allowHalfOpen is set to false, the stream will automatically end the
writable side when the readable side ends, but not the other way around.

Fixes: nodejs#4044
@Trott
Copy link
Member

Trott commented Jul 7, 2017

Doc update PR: #14127

@Trott Trott closed this as completed in 199ad1d Jul 10, 2017
addaleax pushed a commit that referenced this issue Jul 11, 2017
If allowHalfOpen is set to false, the stream will automatically end the
writable side when the readable side ends, but not the other way around.

PR-URL: #14127
Fixes: #4044
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this issue Jul 18, 2017
If allowHalfOpen is set to false, the stream will automatically end the
writable side when the readable side ends, but not the other way around.

PR-URL: #14127
Fixes: #4044
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fishrock123 pushed a commit that referenced this issue Jul 19, 2017
If allowHalfOpen is set to false, the stream will automatically end the
writable side when the readable side ends, but not the other way around.

PR-URL: #14127
Fixes: #4044
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Aug 16, 2017
If allowHalfOpen is set to false, the stream will automatically end the
writable side when the readable side ends, but not the other way around.

PR-URL: #14127
Fixes: #4044
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Sep 5, 2017
If allowHalfOpen is set to false, the stream will automatically end the
writable side when the readable side ends, but not the other way around.

PR-URL: #14127
Fixes: #4044
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants