Skip to content

test: test-timers-unrefd-interval-still-fires.js flaky on smartOS #4559

Closed
Closed
test: test-timers-unrefd-interval-still-fires.js flaky on smartOS#4559
Labels
smartosIssues and PRs related to the SmartOS platform.testIssues and PRs related to Node.js core tests and test infrastructure.timersIssues and PRs related to timers, setImmediate(), setInterval(), and setTimeout().

Description

@MylesBorins
MylesBorins
opened on Jan 7, 2016

Seeing flaky results in LTS

https://ci.nodejs.org/job/node-test-commit-smartos/813/

# [FAIL] Interval fired 5/5 times.
# /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos14-64/test/parallel/test-timers-unrefd-interval-still-fires.js:14
#   throw new Error('Test timed out. keepOpen was not canceled.');
#   ^
# 
# Error: Test timed out. keepOpen was not canceled.
#     at null._onTimeout (/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos14-64/test/parallel/test-timers-unrefd-interval-still-fires.js:14:9)
#     at Timer.listOnTimeout (timers.js:92:15)

https://ci.nodejs.org/job/node-test-commit-smartos/810/

not ok 757 test-timers-unrefd-interval-still-fires.js
# [FAIL] Interval fired 4/5 times.
# /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos14-64/test/parallel/test-timers-unrefd-interval-still-fires.js:14
#   throw new Error('Test timed out. keepOpen was not canceled.');
#   ^
# 
# Error: Test timed out. keepOpen was not canceled.
#     at null._onTimeout (/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos14-64/test/parallel/test-timers-unrefd-interval-still-fires.js:14:9)
#     at Timer.listOnTimeout (timers.js:92:15)

Passes on job 811 and 812 though.

Running one more time to see if it fails again

https://ci.nodejs.org/job/node-test-commit-smartos/814/

Activity

  1. mscdex
    added
    timersIssues and PRs related to timers, setImmediate(), setInterval(), and setTimeout().
    testIssues and PRs related to Node.js core tests and test infrastructure.
    smartosIssues and PRs related to the SmartOS platform.
    on Jan 7, 2016

  2. Trott commented on Jan 7, 2016

    @Trott
    Trott
    on Jan 7, 2016
    Member

    Current version fails stress test: https://ci.nodejs.org/job/node-stress-single-test/307/nodes=smartos14-64/console

    This would appear to be a fulfillment of the prophecy of @misterdjules at #3550 (comment).

    I've refactored the test to not rely on an arbitrary timeout as it is not a performance benchmark but a functionality test designed to check for a very specific bug.

    The refactored test passes the stress test: https://ci.nodejs.org/job/node-stress-single-test/311/nodes=smartos14-64/console

    PR: #4561

  3. Fishrock123 commented on Jan 7, 2016

    @Fishrock123
    Fishrock123
    on Jan 7, 2016
    Contributor

    One of the aforementioned failures is particularly interesting:

    # [FAIL] Interval fired 5/5 times.
# /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos14-64/test/parallel/test-timers-unrefd-interval-still-fires.js:14
#   throw new Error('Test timed out. keepOpen was not canceled.');
#   ^
# 
# Error: Test timed out. keepOpen was not canceled.
#     at null._onTimeout (/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos14-64/test/parallel/test-timers-unrefd-interval-still-fires.js:14:9)
#     at Timer.listOnTimeout (timers.js:92:15)

    As far as I can tell, it isn't possible for this to happen. If it's 5/5 the timeout that throws that error will already be cancelled...

  4. Trott commented on Jan 7, 2016

    @Trott
    Trott
    on Jan 7, 2016
    Member

    @Fishrock123 Perhaps a race condition caused by the clearTimeout(keepOpen) being in a setImmediate() rather than just called directly? I'm not sure that's possible, but since the impossible is happening, something is up.

  5. Fishrock123 commented on Jan 7, 2016

    @Fishrock123
    Fishrock123
    on Jan 7, 2016
    Contributor

    Oh hmmm.

    @Trott I can't find where I suggested that to be added, I forget why that was. It should be removed, I think. (The setImmediate())

  6. Trott commented on Jan 7, 2016

    @Trott
    Trott
    on Jan 7, 2016
    Member

    @Fishrock123 My first attempt at a fix for this was to just remove the setImmediate() (and convert arrow functions to ES5-compatible functions) and leave everything else: b954d83

    It was still flaky when stress tested, though:
    https://ci.nodejs.org/job/node-stress-single-test/308/nodes=smartos14-64/console

    But it did get rid of the 5/5. The failures were all 4/5, 3/5...

    The control stress test (against master) showed 5/5 failures: https://ci.nodejs.org/job/node-stress-single-test/307/nodes=smartos14-64/console

    So yeah, it looks like the setImmediate() caused the 5/5 weirdness. (But fixing that did not fix the test flakiness.)

    At least that mystery is solved...

  7. Trott
    added a commit that references this issue on Jan 9, 2016

    test: fix flaky unrefed timers test

    a43106e
  8. Trott
    closed this as completedin e071894on Jan 11, 2016

  9. Trott commented on Jan 11, 2016

    @Trott
    Trott
    on Jan 11, 2016
    Member

    e071894 should workaround the issue for you, @thealphanerd. There's a more thorough fix that will hopefully land soon, but that should resolve this issue for you if you land it on LTS.

  10. Trott
    added a commit that references this issue on Jan 11, 2016

    test: fix test-timers-unrefd-interval-still-fires

    13529be
  11. MylesBorins
    added 2 commits that reference this issue on Jan 11, 2016

    test: fix flaky unrefed timers test

    36b385d

    test: fix flaky unrefed timers test

    6f9a96f
  12. MylesBorins
    added a commit that references this issue on Jan 28, 2016

    test: fix flaky unrefed timers test

    18e968b
  13. MylesBorins
    added 3 commits that reference this issue on Feb 11, 2016

    test: fix flaky unrefed timers test

    94fbf92

    test: fix flaky unrefed timers test

    c1ba1dc

    test: fix flaky unrefed timers test

    556703d
  14. scovetta
    added a commit that references this issue on Apr 2, 2016

    test: fix flaky unrefed timers test

    8afda3e
  15. sjarmak
    added a commit that references this issue on Jul 27, 2026

    test: fix flaky unrefed timers test

    e071894
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    smartosIssues and PRs related to the SmartOS platform.testIssues and PRs related to Node.js core tests and test infrastructure.timersIssues and PRs related to timers, setImmediate(), setInterval(), and setTimeout().

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions