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

net: make Server.prototype.unref() persistent #897

Closed
wants to merge 1 commit into from
Closed

net: make Server.prototype.unref() persistent #897

wants to merge 1 commit into from

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Feb 19, 2015

Currently, the unref() method does not remember any state if called before the server's handle has been created. This commit adds state to track calls to ref() and unref().

Related to nodejs/node-v0.x-archive#7077

Currently, the unref() method does not remember any state
if called before the server's handle has been created. This
commit adds state to track calls to ref() and unref().
@evanlucas
Copy link
Contributor

LGTM

cjihrig added a commit that referenced this pull request Feb 21, 2015
Currently, the unref() method does not remember any state
if called before the server's handle has been created. This
commit adds state to track calls to ref() and unref().

PR-URL: #897
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Brendan Ashworth <brendan.ashworth@me.com>
@brendanashworth
Copy link
Contributor

Thanks @cjihrig, merged in d8eb974a!

@cjihrig cjihrig deleted the server-unref branch February 21, 2015 04:34
This was referenced Feb 23, 2015
Trott added a commit to Trott/io.js that referenced this pull request Nov 6, 2016
`test-regress-nodejsGH-897` is dependent on a timer firing within a period of
time. Especially on some of the FreeBSD hosts on CI, we have seen tests
like that fail when run in parallel. (This may have nothing to do with
FreeBSD and may just mean that the hosts are resource-constrained.) Move
this test to sequential as we have done with several other
timer-dependent tests recently.

The test has also been refactored and documented via comments.
Trott added a commit to Trott/io.js that referenced this pull request Nov 10, 2016
`test-regress-nodejsGH-897` is dependent on a timer firing within a period of
time. Especially on some of the FreeBSD hosts on CI, we have seen tests
like that fail when run in parallel. (This may have nothing to do with
FreeBSD and may just mean that the hosts are resource-constrained.) Move
this test to sequential as we have done with several other
timer-dependent tests recently.

The test has also been refactored and documented via comments.

PR-URL: nodejs#9487
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
addaleax pushed a commit that referenced this pull request Nov 22, 2016
`test-regress-GH-897` is dependent on a timer firing within a period of
time. Especially on some of the FreeBSD hosts on CI, we have seen tests
like that fail when run in parallel. (This may have nothing to do with
FreeBSD and may just mean that the hosts are resource-constrained.) Move
this test to sequential as we have done with several other
timer-dependent tests recently.

The test has also been refactored and documented via comments.

PR-URL: #9487
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
addaleax pushed a commit to addaleax/node that referenced this pull request Dec 8, 2016
`test-regress-nodejsGH-897` is dependent on a timer firing within a period of
time. Especially on some of the FreeBSD hosts on CI, we have seen tests
like that fail when run in parallel. (This may have nothing to do with
FreeBSD and may just mean that the hosts are resource-constrained.) Move
this test to sequential as we have done with several other
timer-dependent tests recently.

The test has also been refactored and documented via comments.

PR-URL: nodejs#9487
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 20, 2016
`test-regress-GH-897` is dependent on a timer firing within a period of
time. Especially on some of the FreeBSD hosts on CI, we have seen tests
like that fail when run in parallel. (This may have nothing to do with
FreeBSD and may just mean that the hosts are resource-constrained.) Move
this test to sequential as we have done with several other
timer-dependent tests recently.

The test has also been refactored and documented via comments.

PR-URL: #9487
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
`test-regress-GH-897` is dependent on a timer firing within a period of
time. Especially on some of the FreeBSD hosts on CI, we have seen tests
like that fail when run in parallel. (This may have nothing to do with
FreeBSD and may just mean that the hosts are resource-constrained.) Move
this test to sequential as we have done with several other
timer-dependent tests recently.

The test has also been refactored and documented via comments.

PR-URL: #9487
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
`test-regress-GH-897` is dependent on a timer firing within a period of
time. Especially on some of the FreeBSD hosts on CI, we have seen tests
like that fail when run in parallel. (This may have nothing to do with
FreeBSD and may just mean that the hosts are resource-constrained.) Move
this test to sequential as we have done with several other
timer-dependent tests recently.

The test has also been refactored and documented via comments.

PR-URL: #9487
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Trott added a commit to Trott/io.js that referenced this pull request Jan 19, 2017
Even after being moved to `sequential` in
1ce05ad, `test-regress-nodejsGH-897` still
was occasionally flaky on Raspberry Pi devices on CI.

The test is especially sensitive to resource constraints. It failed
reliably on my laptop if I moved it to `parallel` and ran 32 competing
node test processes. Even for a flaky test, that's unusually low. I
typically don't see problems, even for flaky tests, until I get up to
around four times that number.

On a Raspberry Pi, of course, that sensitivity to resource constraints
will manifest much sooner.

This change checks the order of timers firing, rather than the duration
before a timer is fired. This eliminates the sensitivity to resource
constraints. The test can now be moved back to `parallel`. I am able to
run many copies of the test simultaneously without seeing test failures.

Fixes: nodejs#10073
Trott added a commit to Trott/io.js that referenced this pull request Jan 23, 2017
Even after being moved to `sequential` in
1ce05ad, `test-regress-nodejsGH-897` still
was occasionally flaky on Raspberry Pi devices on CI.

The test is especially sensitive to resource constraints. It failed
reliably on my laptop if I moved it to `parallel` and ran 32 competing
node test processes. Even for a flaky test, that's unusually low. I
typically don't see problems, even for flaky tests, until I get up to
around four times that number.

On a Raspberry Pi, of course, that sensitivity to resource constraints
will manifest much sooner.

This change checks the order of timers firing, rather than the duration
before a timer is fired. This eliminates the sensitivity to resource
constraints. The test can now be moved back to `parallel`. I am able to
run many copies of the test simultaneously without seeing test failures.

PR-URL: nodejs#10903
Fixes: nodejs#10073
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@Trott Trott mentioned this pull request Jan 23, 2017
3 tasks
Trott added a commit to Trott/io.js that referenced this pull request Jan 23, 2017
In test-timers, confirm that all input values that should be coerced to
1 ms are not being coerced to a significantly larger value.

This eliminates the need for the separate test-regress-nodejsGH-897.
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 25, 2017
Even after being moved to `sequential` in
1ce05ad, `test-regress-nodejsGH-897` still
was occasionally flaky on Raspberry Pi devices on CI.

The test is especially sensitive to resource constraints. It failed
reliably on my laptop if I moved it to `parallel` and ran 32 competing
node test processes. Even for a flaky test, that's unusually low. I
typically don't see problems, even for flaky tests, until I get up to
around four times that number.

On a Raspberry Pi, of course, that sensitivity to resource constraints
will manifest much sooner.

This change checks the order of timers firing, rather than the duration
before a timer is fired. This eliminates the sensitivity to resource
constraints. The test can now be moved back to `parallel`. I am able to
run many copies of the test simultaneously without seeing test failures.

PR-URL: nodejs#10903
Fixes: nodejs#10073
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Trott added a commit that referenced this pull request Jan 26, 2017
In test-timers, confirm that all input values that should be coerced to
1 ms are not being coerced to a significantly larger value.

This eliminates the need for the separate test-regress-GH-897.

PR-URL: #10960
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 27, 2017
Even after being moved to `sequential` in
1ce05ad, `test-regress-nodejsGH-897` still
was occasionally flaky on Raspberry Pi devices on CI.

The test is especially sensitive to resource constraints. It failed
reliably on my laptop if I moved it to `parallel` and ran 32 competing
node test processes. Even for a flaky test, that's unusually low. I
typically don't see problems, even for flaky tests, until I get up to
around four times that number.

On a Raspberry Pi, of course, that sensitivity to resource constraints
will manifest much sooner.

This change checks the order of timers firing, rather than the duration
before a timer is fired. This eliminates the sensitivity to resource
constraints. The test can now be moved back to `parallel`. I am able to
run many copies of the test simultaneously without seeing test failures.

PR-URL: nodejs#10903
Fixes: nodejs#10073
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this pull request Jan 28, 2017
In test-timers, confirm that all input values that should be coerced to
1 ms are not being coerced to a significantly larger value.

This eliminates the need for the separate test-regress-GH-897.

PR-URL: #10960
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@italoacasas italoacasas mentioned this pull request Jan 29, 2017
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 30, 2017
Even after being moved to `sequential` in
1ce05ad, `test-regress-nodejsGH-897` still
was occasionally flaky on Raspberry Pi devices on CI.

The test is especially sensitive to resource constraints. It failed
reliably on my laptop if I moved it to `parallel` and ran 32 competing
node test processes. Even for a flaky test, that's unusually low. I
typically don't see problems, even for flaky tests, until I get up to
around four times that number.

On a Raspberry Pi, of course, that sensitivity to resource constraints
will manifest much sooner.

This change checks the order of timers firing, rather than the duration
before a timer is fired. This eliminates the sensitivity to resource
constraints. The test can now be moved back to `parallel`. I am able to
run many copies of the test simultaneously without seeing test failures.

PR-URL: nodejs#10903
Fixes: nodejs#10073
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 30, 2017
In test-timers, confirm that all input values that should be coerced to
1 ms are not being coerced to a significantly larger value.

This eliminates the need for the separate test-regress-nodejsGH-897.

PR-URL: nodejs#10960
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 30, 2017
Even after being moved to `sequential` in
1ce05ad, `test-regress-nodejsGH-897` still
was occasionally flaky on Raspberry Pi devices on CI.

The test is especially sensitive to resource constraints. It failed
reliably on my laptop if I moved it to `parallel` and ran 32 competing
node test processes. Even for a flaky test, that's unusually low. I
typically don't see problems, even for flaky tests, until I get up to
around four times that number.

On a Raspberry Pi, of course, that sensitivity to resource constraints
will manifest much sooner.

This change checks the order of timers firing, rather than the duration
before a timer is fired. This eliminates the sensitivity to resource
constraints. The test can now be moved back to `parallel`. I am able to
run many copies of the test simultaneously without seeing test failures.

PR-URL: nodejs#10903
Fixes: nodejs#10073
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 30, 2017
In test-timers, confirm that all input values that should be coerced to
1 ms are not being coerced to a significantly larger value.

This eliminates the need for the separate test-regress-nodejsGH-897.

PR-URL: nodejs#10960
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 8, 2017
Even after being moved to `sequential` in
1ce05ad, `test-regress-GH-897` still
was occasionally flaky on Raspberry Pi devices on CI.

The test is especially sensitive to resource constraints. It failed
reliably on my laptop if I moved it to `parallel` and ran 32 competing
node test processes. Even for a flaky test, that's unusually low. I
typically don't see problems, even for flaky tests, until I get up to
around four times that number.

On a Raspberry Pi, of course, that sensitivity to resource constraints
will manifest much sooner.

This change checks the order of timers firing, rather than the duration
before a timer is fired. This eliminates the sensitivity to resource
constraints. The test can now be moved back to `parallel`. I am able to
run many copies of the test simultaneously without seeing test failures.

PR-URL: #10903
Fixes: #10073
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
jasnell pushed a commit that referenced this pull request Mar 8, 2017
In test-timers, confirm that all input values that should be coerced to
1 ms are not being coerced to a significantly larger value.

This eliminates the need for the separate test-regress-GH-897.

PR-URL: #10960
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
jasnell pushed a commit that referenced this pull request Mar 8, 2017
In test-timers, confirm that all input values that should be coerced to
1 ms are not being coerced to a significantly larger value.

This eliminates the need for the separate test-regress-GH-897.

PR-URL: #10960
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
Even after being moved to `sequential` in
1ce05ad, `test-regress-GH-897` still
was occasionally flaky on Raspberry Pi devices on CI.

The test is especially sensitive to resource constraints. It failed
reliably on my laptop if I moved it to `parallel` and ran 32 competing
node test processes. Even for a flaky test, that's unusually low. I
typically don't see problems, even for flaky tests, until I get up to
around four times that number.

On a Raspberry Pi, of course, that sensitivity to resource constraints
will manifest much sooner.

This change checks the order of timers firing, rather than the duration
before a timer is fired. This eliminates the sensitivity to resource
constraints. The test can now be moved back to `parallel`. I am able to
run many copies of the test simultaneously without seeing test failures.

PR-URL: #10903
Fixes: #10073
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
In test-timers, confirm that all input values that should be coerced to
1 ms are not being coerced to a significantly larger value.

This eliminates the need for the separate test-regress-GH-897.

PR-URL: #10960
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Mar 9, 2017
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
In test-timers, confirm that all input values that should be coerced to
1 ms are not being coerced to a significantly larger value.

This eliminates the need for the separate test-regress-GH-897.

PR-URL: #10960
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
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.

3 participants