src: fix sporadic deadlock in SIGUSR1 handler #7675
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add a basic regression test that checks if the map for IncomingMessage and OutgoingMessage objects is stable over time. The test is not exhaustive in that it doesn't try to establish whether the transition path is the same on every request, it just checks that objects in their final states have the same map. To be investigated why the first (and only the first) ServerRequest object ends up with a deprecated map, regardless of the number of iterations. PR-URL: nodejs#7003 Refs: nodejs#6294 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Fixes: nodejs#6925 PR-URL: nodejs#7270 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Refs: nodejs#6655 PR-URL: nodejs#6694 Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ingvar Stepanyan <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> PR-URL: nodejs#7407
PR-URL: nodejs#7320 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Rod Vagg <[email protected]>
PR-URL: nodejs#7364 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
Clarify how the encoding option interacts with the data type of child process stdout and stderr. Fixes: nodejs#6666 PR-URL: nodejs#7361 Reviewed-By: Colin Ihrig <[email protected]>
Fix typo in example PR-URL: nodejs#7411 Reviewed-By: Brian White <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: James M Snell <[email protected]>
Remove special handling when asserting on a pair of arguments objects. The code being removed will only run if both `expected` and `actual` are arguments objects. Given that situation, the subsequent code for handling everything else works just fine. Tests added to confirm expected behavior. This came about while trying to improve test coverage. The segment of code removed had no test coverage. I was unable to write a test that would both exercise the code and fail if the code was removed. Further examination indicated that this was because the special handling was not needed. PR-URL: nodejs#7413 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#7466 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Bryan English <[email protected]> Reviewed-By: Brian White <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#7467 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Brian White <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
The example changed by this commit uses ['ignore'] where 'ignore' is intended. Fixes: nodejs#7269 PR-URL: nodejs#7540 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: João Reis <[email protected]> PR-URL: nodejs#7567
PR-URL: nodejs#7528 Reviewed-By: Anna Henningsen <[email protected]>
Increase socket timeout so that there is enough time to reliably run the test on FreeBSD. Fixes: nodejs#7516 PR-URL: nodejs#7555 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>
Added clarification about the linter execution. PR-URL: nodejs#7534 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Add benchmark to "Who to CC". Also, alphabetized the only non-alphabetized subsystem. PR-URL: nodejs#7604 Reviewed-By: Ben Noordhuis <[email protected]>
The dns.resolve documentation stated that an array of IP addresses would be returned in the callback. This is true for everything other than the SOA record which returns an object. This fixes that documentation. Fixes: nodejs#6506 PR-URL: nodejs#7532 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Roman Reiss <[email protected]>
`oldDirs` is assigned but never used. Remove it. (This was missed by the linter in previous versions of ESLint but is flagged by the current version. Updating the linter is contingent on this change or some similar remedy landing.) PR-URL: nodejs#7594 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
`connections` is assigned but never used. Remove it. (This was missed by the linter in previous versions of ESLint but is flagged by the current version. Updating the linter is contingent on this change or some similar remedy landing.) PR-URL: nodejs#7595 Reviewed-By: Colin Ihrig <[email protected]>
`messageCount` is assigned, but never used. Remove it. (This was missed by the linter in previous versions of ESLint but is flagged by the current version. Updating the linter is contingent on this change or some similar remedy landing.) PR-URL: nodejs#7599 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
`writes` is assigned but never used. Remove it. (This was missed by the linter in previous versions of ESLint but is flagged by the current version. Updating the linter is contingent on this change or some similar remedy landing.) PR-URL: nodejs#7596 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
`connections` is assigned but never used. Remove it. (This was missed by the linter in previous versions of ESLint but is flagged by the current version. Updating the linter is contingent on this change or some similar remedy landing.) PR-URL: nodejs#7597 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
Remove variables that are assigned but never used. (This was missed by the linter in previous versions of ESLint but is flagged by the current version. Updating the linter is contingent on this change or some similar remedy landing.) PR-URL: nodejs#7600 Reviewed-By: Rod Vagg <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
Remove handful of variables that are assigned but never used. (This was missed by the linter in previous versions of ESLint but is flagged by the current version. Updating the linter is contingent on this change or some similar remedy landing.) PR-URL: nodejs#7598 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
Update ESLint to 3.0.0. This includes an enhancement to `no-unused-vars` such that it finds a few instances in our code base that it did not find previously (fixed in previous commits readying this for landing). PR-URL: nodejs#7601 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Roman Reiss <[email protected]>
Previous version of weak used for gc tests emitted a warning on OS X. Updating to current version eliminates warning. PR-URL: nodejs#7014 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
Code coverage information shows that we are only testing the happy path for the internal readline `isFullWidthCodePoint()` function. Test it with invalid input. PR-URL: nodejs#7422 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Brian White <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
This commit adds the missing handle argument to the cluster worker 'message' event. It also adds a link to the process 'message' event for reference. Refs: nodejs#7297 PR-URL: nodejs#7309 Reviewed-By: Brian White <[email protected]>
strcasecmp() is not used in src/node_http_parser.cc so there is no need to include its header file. PR-URL: nodejs#6582 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: James M Snell <[email protected]>
strcasecmp() is affected by the current locale as configured through e.g. the LC_ALL environment variable and the setlocale() libc function. It can result in unpredictable results across systems so replace it with a function that isn't susceptible to that. PR-URL: nodejs#6582 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: James M Snell <[email protected]>
This is the certdata.txt[0] that ships in Firefox 47 and NSS 3.23, last updated on 2016-02-26. [0] https://hg.mozilla.org/mozilla-central/raw-file/1f84dea6508d/security/nss/lib/ckfw/builtins/certdata.txt PR-URL: nodejs#7363 Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Update the list of root certificates in src/node_root_certs.h with tools/mk-ca-bundle.pl. Certificates added: - CA WoSign ECC Root - Certification Authority of WoSign G2 - Certinomis - Root CA - Certum Trusted Network CA 2 - OISTE WISeKey Global Root GB CA - SZAFIR ROOT CA2 - TURKTRUST Elektronik Sertifika Hizmet Sa?layıcısı H5 - TURKTRUST Elektronik Sertifika Hizmet Sa?layıcısı H6 Certificates removed: - A-Trust-nual-03 - Buypass Class 3 CA 1 - CA Disig - ComSign Secured CA - Equifax Secure CA - NetLock Notary (Class A) Root - Staat der Nederlanden Root CA - TC TrustCenter Class 2 CA II - TC TrustCenter Universal CA I - TURKTRUST Certificate Services Provider Root 1 - TURKTRUST Certificate Services Provider Root 2 - UTN DATACorp SGC Root CA - Verisign Class 4 Public Primary Certification Authority - G3 PR-URL: nodejs#7363 Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Shigeki Ohtsu points out that the test is unreliable because some of the www1.cnnnic.cn servers are misconfigured. Remove it. PR-URL: nodejs#7363 Refs: nodejs#7363 (comment) Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Pointed out by Coverity. PR-URL: nodejs#7374 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
This commit adds a CHECK that verifies that the file event watcher is not started twice, which would be indicative of a bug in lib/fs.js. PR-URL: nodejs#7374 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Remove TLSWrap::write_queue_size_, it's not used anywhere. PR-URL: nodejs#7374 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
The code assigned the result of EVP_get_digestbyname() to data members called md_ that were not used outside the initialization functions. PR-URL: nodejs#7374 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
PR-URL: nodejs#7374 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
PR-URL: nodejs#7374 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Pointed out by Coverity. Introduced in commit 5b8e1da from September 2011 ("Initial pass at zlib bindings".) The asynchronous version of Write() used a pointer to a stack-allocated buffer on flush. A mitigating factor is that zlib does not dereference the pointer for zero-sized writes but it's still technically UB. PR-URL: nodejs#7374 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Pointed out by Coverity. Introduced in commits 3546383 ("process_wrap: avoid leaking memory when throwing due to invalid arguments") and fa4eb47 ("bindings: add spawn_sync bindings"). The return statements inside the if blocks were dead code because their guard conditions always evaluated to false. Remove them. PR-URL: nodejs#7374 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Catch and emit `certCbDone` exceptions instead of throwing them as `uncaughtException` and crashing the whole process. Fix: nodejs#6822 PR-URL: nodejs#6887 Reviewed-By: Ben Noordhuis <[email protected]>
The test targets expect that V8 is built in deps/v8/out Ref: nodejs#7477 PR-URL: nodejs#7482 Reviewed-By: Myles Borins <[email protected]> Reviewed-By: James M Snell <[email protected]>
Without this it would always compile Release and Debug builds. Ref: nodejs#7477 PR-URL: nodejs#7482 Reviewed-By: Myles Borins <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs-github-bot
added
the
c++
This was referenced Jul 12, 2016
Calling v8::Debug::DebugBreak() directly from the signal handler is unsafe because said function tries to grab a mutex. Work around that by starting a watchdog thread that is woken up from the signal handler, which then calls v8::Debug::DebugBreak(). Using a watchdog thread also removes the need to use atomic operations so this commit does away with that. Fixes: nodejs#5368 PR-URL: nodejs#5904 Reviewed-By: Anna Henningsen <[email protected]>
bnoordhuis
force-pushed
the
backport-pr5904-v4.x
branch
from
9a02ed8 to
2b81f80
Compare
MylesBorins
force-pushed
the
v4.x-staging
branch
from
fcdb93d to
27222f4
Compare
|
landed in f886b01 This PR looks pretty gnarly now since i was doing some rebasing. I manually rebased and landed the commits locally |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
R=@thealphanerd
CI: https://ci.nodejs.org/job/node-test-pull-request/3265/
EDIT: https://ci.nodejs.org/job/node-test-pull-request/3267/ - botched the rebase