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

doc: os.uptime() temporary bug notice #36503

Closed
wants to merge 6 commits into from
Closed

doc: os.uptime() temporary bug notice #36503

wants to merge 6 commits into from

Conversation

schamberg97
Copy link
Contributor

Checklist

This commit makes it clear that os.uptime() is currently unreliable in some virtualization cases, until libuv/libuv#3068 is resolved (and thus #36244). At least one member of Node.js (@benjamingr) in the aforementioned issue appears to be in favour of at least a temporary notice, until the issue is resolved by libuv.

However, since similar issues may reappear in the future with the evolution of virtualization technology, I believe it is prudent to leave a permanent notice on os.uptime() doc, so perhaps it is also worth to consider.

Makes it clear that os.uptime() is unreliable in some virtualization cases.
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. os Issues and PRs related to the os subsystem. labels Dec 13, 2020
@schamberg97 schamberg97 changed the title os.uptime() temporary bug notice doc: os.uptime() temporary bug notice Dec 13, 2020
@mscdex
Copy link
Contributor

mscdex commented Dec 14, 2020

This seems unnecessary given the linked libuv issue was created only 2 hours ago. Is there any indication that libuv won't fix this in a reasonable amount of time?

@schamberg97
Copy link
Contributor Author

This seems unnecessary given the linked libuv issue was created only 2 hours ago. Is there any indication that libuv won't fix this in a reasonable amount of time?

In my belief, libuv maintainers may need to spend some time coming to a decision on how best to resolve the issue. I am just unsure how much time it may require. But perhaps you are right, in which case this PR should probably remain open without any further action until there is better understanding of how things develop.

doc/api/os.md Outdated Show resolved Hide resolved
doc/api/os.md Outdated
Please be aware that the value returned can be inaccurate in some
rare virtualization cases. The issue arises when the virtualized
guest instance shares the kernel with the host system due to a bug
in libuv. `os.uptime()` may thus provide the host's uptime instead of guest's.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way this sentence is worded it implies that the bug in libuv is what is responsible for the guest instance sharing the kernel with the host system.

schamberg97 and others added 2 commits December 14, 2020 20:06
doc/api/os.md Outdated Show resolved Hide resolved
doc/api/os.md Outdated Show resolved Hide resolved
schamberg97 and others added 2 commits December 19, 2020 16:22
@schamberg97
Copy link
Contributor Author

@Trott thanks for review 😊 I am unfortunately always absent-minded about linting markdown. Gotta do something about it 🤓

@schamberg97
Copy link
Contributor Author

schamberg97 commented Dec 19, 2020

Regarding the PR, despite some opinions to the contrary, I still think it needs to be accepted, because libuv guys seem to be somewhat slow with reviewing the fix, perhaps due to the season 😇 But hopefully it’ll be resolved soon

jasnell pushed a commit that referenced this pull request Jan 9, 2021
Makes it clear that os.uptime() is unreliable in some virtualization
cases.

PR-URL: #36503
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@jasnell
Copy link
Member

jasnell commented Jan 9, 2021

Landed in b9ffb82

@jasnell jasnell closed this Jan 9, 2021
danielleadams pushed a commit that referenced this pull request Jan 12, 2021
Makes it clear that os.uptime() is unreliable in some virtualization
cases.

PR-URL: #36503
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@danielleadams danielleadams mentioned this pull request Jan 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. os Issues and PRs related to the os subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants