-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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: remove process listeners after stopping the server #4013
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4013 +/- ##
==========================================
+ Coverage 92.62% 92.74% +0.12%
==========================================
Files 14 14
Lines 1410 1420 +10
Branches 519 524 +5
==========================================
+ Hits 1306 1317 +11
+ Misses 96 95 -1
Partials 8 8
Continue to review full report at Codecov.
|
a186750
to
447fe4d
Compare
lib/Server.js
Outdated
// We add listeners to signals when creating a new Server instance | ||
// So ensure they are removed to prevent EventEmitter memory leak warnings | ||
process.removeAllListeners("SIGINT"); | ||
process.removeAllListeners("SIGTERM"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not remove all of them just only our, move our close function above the file and use removeListener
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
above where?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somewhere here https://github.com/webpack/webpack-dev-server/blob/master/lib/Server.js#L17, like
if (needForceShutdown) {
exitProcess();
}
this.logger.info(
"Gracefully shutting down. To force exit, press ^C again. Please wait..."
);
needForceShutdown = true;
this.stopCallback(() => {
if (typeof this.compiler.close === "function") {
this.compiler.close(exitProcess);
} else {
exitProcess();
}
});
from https://github.com/webpack/webpack-dev-server/blob/master/lib/Server.js#L1177
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I.e. just move it in function above the Server class and remove this listener in close
For Bugs and Features; did you add new tests?
No
Motivation / Use-Case
fix memory leak warnings in e2e tests.
Breaking Changes
None
Additional Info
No