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

Reproductible ERR_INTERNAL_ASSERTION on node 15 #38189

Closed
Congelli501 opened this issue Apr 10, 2021 · 5 comments
Closed

Reproductible ERR_INTERNAL_ASSERTION on node 15 #38189

Congelli501 opened this issue Apr 10, 2021 · 5 comments
Labels
confirmed-bug Issues with confirmed bugs. stream Issues and PRs related to the stream subsystem.

Comments

@Congelli501
Copy link

Congelli501 commented Apr 10, 2021

What steps will reproduce the bug?

I had a ERR_INTERNAL_ASSERTION when trying to run our application using node 15.x, for a stream destroy test. This code is the minimal version to reproduce it.
The code works ok on node 14.16.1, but fails under node 15.14.0

'use strict';

const {pipeline: pipelineCb, Writable, Readable} = require('stream');
const util = require('util');
const pipeline = util.promisify(pipelineCb);

(async () => {
	// Create a dummy readable stream
	let i = 0;
	const readable = new Readable({
		objectMode: true,
		read () {
			this.push({counter: i++});
		}
	});

	// Create a dummy writable stream with backpressure
	const writable = new Writable({
		objectMode: true,
		write: (chunk, encoding, callback) => {
			setTimeout(callback, 10); // Backpressure
		}
	});

	// Pipeline the sql stream in the dummy writable stream & destroy the writable stream after a few ms
	try {
		await Promise.all([
			pipeline(readable, writable),
			(async () => {
				// Destroy the stream after a few element
				// pipeline will destroy all the streams
				await new Promise(resolve => setTimeout(resolve, 50));
				writable.destroy();
			})()
		]);
	} catch (e) {
		if (e.code !== 'ERR_STREAM_PREMATURE_CLOSE') {
			throw e;
		}
	}
})().catch(e => {
	console.error(e);
	process.exitCode = 1;
});

To reproduce (under node 15):

% node index.js
node:internal/assert:14
    throw new ERR_INTERNAL_ASSERTION(message);
    ^

Error [ERR_INTERNAL_ASSERTION]: This is caused by either a bug in Node.js or incorrect usage of Node.js internals.
Please open an issue with this stack trace at https://github.com/nodejs/node/issues

    at new NodeError (node:internal/errors:329:5)
    at assert (node:internal/assert:14:11)
    at Writable.destroy (node:internal/streams/writable:851:5)
    at Object.destroyer (node:internal/streams/destroy:365:59)
    at node:internal/streams/pipeline:73:17
    at finish (node:internal/streams/pipeline:159:23)
    at node:internal/util:408:5
    at node:internal/streams/pipeline:74:5
    at finish (node:internal/streams/pipeline:159:23)
    at node:internal/util:408:5 {
  code: 'ERR_INTERNAL_ASSERTION'
}

Expected behavior (under node14):

% node index.js
<no output>
@aduh95 aduh95 added the confirmed-bug Issues with confirmed bugs. label Apr 10, 2021
@aduh95
Copy link
Contributor

aduh95 commented Apr 10, 2021

Can reproduce on master. Also happens when using pipeline from stream/promises and setTimeout from timers/promises.

@aduh95 aduh95 added the stream Issues and PRs related to the stream subsystem. label Apr 10, 2021
@Linkgoron
Copy link
Member

Linkgoron commented Apr 10, 2021

The assert was added here: #35067

I think that there might be two issues here:

  • pipeline probably shouldn't destroy an already destroyed stream
  • writable maybe (?) not changing the inner state correctly when the state is "writing", or maybe the assert should be skipped if it's still writing

@Linkgoron
Copy link
Member

Linkgoron commented Apr 11, 2021

A reproduction without using pipeline:

const { Writable } = require('stream');

// Create a dummy writable stream with backpressure
const writable = new Writable({
  objectMode: true,
  write: (chunk, encoding, callback) => {
    setTimeout(callback, 100); // Backpressure
  }
});
writable.write('a');
writable.write('b');
writable.destroy();
writable.destroy();

@lpinca
Copy link
Member

lpinca commented Apr 12, 2021

cc: @nodejs/streams @ronag

@ronag
Copy link
Member

ronag commented Apr 12, 2021

I'll take a look.

ronag added a commit to nxtedition/node that referenced this issue Apr 12, 2021
Calling Writable.destroy() multiple times in the same tick
could cause an assertion error.

Fixes: nodejs#38189
ronag added a commit to nxtedition/node that referenced this issue Apr 12, 2021
Calling Writable.destroy() multiple times in the same tick
could cause an assertion error.

Fixes: nodejs#38189
@ronag ronag closed this as completed in 369f239 Apr 16, 2021
ronag added a commit to nxtedition/node that referenced this issue Apr 29, 2021
Calling Writable.destroy() multiple times in the same tick
could cause an assertion error.

Fixes: nodejs#38189

PR-URL: nodejs#38221
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Nitzan Uziely <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
ronag added a commit to nxtedition/node that referenced this issue Apr 29, 2021
Calling Writable.destroy() multiple times in the same tick
could cause an assertion error.

Fixes: nodejs#38189

PR-URL: nodejs#38221
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Nitzan Uziely <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
ronag added a commit to nxtedition/node that referenced this issue Apr 29, 2021
Calling Writable.destroy() multiple times in the same tick
could cause an assertion error.

Fixes: nodejs#38189

PR-URL: nodejs#38221
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Nitzan Uziely <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Backport-PR-URL: nodejs#38473
ronag added a commit to nxtedition/node that referenced this issue Apr 29, 2021
Calling Writable.destroy() multiple times in the same tick
could cause an assertion error.

Fixes: nodejs#38189

PR-URL: nodejs#38221
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Nitzan Uziely <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Backport-PR-URL: nodejs#38473
ronag added a commit to nxtedition/node that referenced this issue Apr 29, 2021
Calling Writable.destroy() multiple times in the same tick
could cause an assertion error.

Fixes: nodejs#38189

PR-URL: nodejs#38221
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Nitzan Uziely <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Backport-PR-URL: nodejs#38473
targos pushed a commit that referenced this issue May 30, 2021
Calling Writable.destroy() multiple times in the same tick
could cause an assertion error.

Fixes: #38189

PR-URL: #38221
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Nitzan Uziely <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Backport-PR-URL: #38473
targos pushed a commit that referenced this issue Jun 5, 2021
Calling Writable.destroy() multiple times in the same tick
could cause an assertion error.

Fixes: #38189

PR-URL: #38221
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Nitzan Uziely <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Backport-PR-URL: #38473
targos pushed a commit that referenced this issue Jun 5, 2021
Calling Writable.destroy() multiple times in the same tick
could cause an assertion error.

Fixes: #38189

PR-URL: #38221
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Nitzan Uziely <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Backport-PR-URL: #38473
targos pushed a commit that referenced this issue Jun 11, 2021
Calling Writable.destroy() multiple times in the same tick
could cause an assertion error.

Fixes: #38189

PR-URL: #38221
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Nitzan Uziely <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Backport-PR-URL: #38473
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants