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

fix: fix memory leak after writing data with false result #542

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

harlentan
Copy link

There will be memory leak when sending big apk to Android device, because the waitForDrain will never be called.

if (this.connection.write(chunk, track)) {

if (this.connection.write(chunk, track)) {
  return writeNext();
} else {
  return waitForDrain().then(writeNext);
}

The if (this.connection.write(chunk, track)) will always be true,

public write(data: Buffer, callback?: (err?: Error) => void): this {

public write(data: Buffer, callback?: (err?: Error) => void): this {
    this.socket.write(dump(data), callback);
    return this;
 }

Nodejs AP reference:

https://nodejs.org/api/stream.html#writablewritechunk-encoding-callback

callback will be called with the error as its first argument. The callback is called asynchronously and before 'error' is emitted.
The return value is true if the internal buffer is less than the highWaterMark configured when the stream was created after admitting chunk. If false is returned, further attempts to write data to the stream should stop until the 'drain' event is emitted.
While a stream is not draining, calls to write() will buffer chunk, and return false. Once all currently buffered chunks are drained (accepted for delivery by the operating system), the 'drain' event will be emitted. Once write() returns false, do not write more chunks until the 'drain' event is emitted. While calling write() on a stream that is not draining is allowed, Node.js will buffer all written chunks until maximum memory usage occurs, at which point it will abort unconditionally. Even before it aborts, high memory usage will cause poor garbage collector performance and high RSS (which is not typically released back to the system, even after the memory is no longer required). Since TCP sockets may never drain if the remote peer does not read the data, writing a socket that is not draining may lead to a remotely exploitable vulnerability.

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

Successfully merging this pull request may close these issues.

1 participant