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

Memory leaks in asynchronous image saving methods #1296

Closed
Agamnentzar opened this issue Nov 4, 2018 · 3 comments · Fixed by #1331
Closed

Memory leaks in asynchronous image saving methods #1296

Agamnentzar opened this issue Nov 4, 2018 · 3 comments · Fixed by #1331
Assignees

Comments

@Agamnentzar
Copy link
Contributor

Issue

Memory leaks appear in asynchronous version of Canvas.toBuffer method and Canvas.createPNGStream method

  • Canvas.toBuffer causes large memory leak
  • Canvas.createPNGStream causes small memory leak

Steps to Reproduce

const fs = require('fs');
const { createCanvas, Image } = require('canvas');

const image = new Image();
image.src = fs.readFileSync('some_test_image.png');

const canvas = createCanvas(image.width, image.height);
canvas.getContext('2d').drawImage(image, 0, 0);

function saveUsingToBuffer(path, canvas) {
	return new Promise((resolve, reject) => {
		canvas.toBuffer((err, buffer) => {
			if (err) {
				reject(err);
			} else {
				fs.writeFile(path, buffer, err => {
					if (err) {
						reject(err);
					} else {
						resolve();
					}
				});
			}
		});
	});
}

function saveUsingSyncToBuffer(path, canvas) {
	return new Promise((resolve, reject) => {
		const buffer = canvas.toBuffer();
		fs.writeFile(path, buffer, err => {
			if (err) {
				reject(err);
			} else {
				resolve();
			}
		});
	});
}

function saveUsingStream(path, canvas) {
	return new Promise((resolve, reject) => {
		const out = fs.createWriteStream(path);
		const stream = canvas.createPNGStream();
		stream.pipe(out);
		out.on('finish', resolve);
		out.on('error', reject);
	});
}

async function test() {
	while (true) {
		// uncomment one test to run
		// await saveUsingSyncToBuffer('test.png', canvas); // synchronous toBuffer() (no leak)
		await saveUsingToBuffer('test.png', canvas); // asynchronous toBuffer() (large leak)
		// await saveUsingStream('test.png', canvas); // createPNGStream() (small leak)
		gc();
	}
}

test();

You need to provide some test png image to load some_test_image.png
The script will keep saving test.png image in current path
Run with --expose-gc flag (or comment out call to gc()

Your Environment

  • Version of node-canvas: 2.0.1
  • Environment: node 9.11.2 on Windows 10 x64
@zbjornson zbjornson added the Bug label Dec 4, 2018
@zbjornson zbjornson self-assigned this Dec 4, 2018
@zbjornson
Copy link
Collaborator

Confirmed, thanks for the clear repro.

Started working on it here master...zbjornson:zb/1296 but there's some heap corruption going on. Sort of uncovered a mess.

@ghost
Copy link

ghost commented Dec 4, 2018

Do you know if these issues also effect the asynchronous version of loadImage?

@Agamnentzar
Copy link
Contributor Author

@Lexxxtentacion just tested sync and async loading of images and it doesn't seem to be affected by any memory leaks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants