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

process.uptime() not correct when adjust time #2573

Closed
yjhjstz opened this issue Aug 27, 2015 · 10 comments
Closed

process.uptime() not correct when adjust time #2573

yjhjstz opened this issue Aug 27, 2015 · 10 comments
Labels
process Issues and PRs related to the process subsystem.

Comments

@yjhjstz
Copy link

yjhjstz commented Aug 27, 2015

Now , iojs use relative time to compute uptime[1], but when call settimeofday to change time,
process.time() isn't correct. So, my PR #2572 is to slove this, using time that refer to the kernel boot time.
That's always ok at most platforms.

refer [1]: https://en.wikipedia.org/wiki/Uptime

@ChALkeR ChALkeR added the process Issues and PRs related to the process subsystem. label Aug 27, 2015
@bnoordhuis
Copy link
Member

but when call settimeofday to change time, process.time isn't correct

process.time is a typo of process.uptime?

On what platform are you seeing this? I'm not sure I understand, libuv uses monotonic clocks - i.e. clocks that are unaffected by settimeofday - whenever possible.

@yjhjstz
Copy link
Author

yjhjstz commented Aug 27, 2015

I mean js-function process.time() implemented in static void Uptime(...).

diff --git a/src/node.cc b/src/node.cc
index b900388..1751f09 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -1972,10 +1972,10 @@ static void Uptime(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   double uptime;

-  uv_update_time(env->event_loop());
-  uptime = uv_now(env->event_loop()) - prog_start_time;
+  uv_uptime(&uptime);
+  uptime -= prog_start_time;

-  args.GetReturnValue().Set(Number::New(env->isolate(), uptime / 1000));
+  args.GetReturnValue().Set(Number::New(env->isolate(), uptime));
 }

@@ -3606,8 +3606,7 @@ void Init(int* argc,
           int* exec_argc,
           const char*** exec_argv) {
   // Initialize prog_start_time to get relative uptime.
-  prog_start_time = static_cast<double>(uv_now(uv_default_loop()));
-
+  uv_uptime(&prog_start_time);
   // Make inherited handles noninheritable.

uv_now() can be affected by settimeofday, so I use uv_uptime(). @bnoordhuis

@bnoordhuis
Copy link
Member

I mean js-function process.time() implemented in static void Uptime(...).

There is no process.time() but I understand that you mean process.uptime(). I ask because there is also process.hrtime() which does something else.

uv_now() can be affected by settimeofday, so I use uv_uptime().

Can you explain how? Like I said, libuv uses monotonic clocks (think clock_gettime(CLOCK_MONOTONIC)) on most platforms. You should never see the uptime jump backwards, although it is possible for a monotonic clock to slow down in the presence of time adjustments.

On some platforms it doesn't matter if you use uv_now() or uv_uptime() because they both call clock_gettime(). With that in mind, can you explain more about the exact behavior you're seeing and on what platform?

@yjhjstz
Copy link
Author

yjhjstz commented Aug 27, 2015

in netbsd,

int uv_uptime(double* uptime) {
  time_t now;
  struct timeval info;
  size_t size = sizeof(info);
  static int which[] = {CTL_KERN, KERN_BOOTTIME};

  if (sysctl(which, 2, &info, &size, NULL, 0))
    return -errno;

  now = time(NULL);

  *uptime = (double)(now - info.tv_sec);
  return 0;
}
uint64_t uv__hrtime(uv_clocktype_t type) {
  struct timespec ts;
  clock_gettime(CLOCK_MONOTONIC, &ts);
  return (((uint64_t) ts.tv_sec) * NANOSEC + ts.tv_nsec);
}

I want to know, why uv_uptime not use clock_gettime, but to use sysctl?

@bnoordhuis
Copy link
Member

I don't know the exact reason. Someone (@shigeki?) contributed that code a long time ago.

@mscdex
Copy link
Contributor

mscdex commented Aug 27, 2015

All of the BSDs use the same uv_uptime() implementation in libuv. process.uptime() uses a monotonic clock via uv_hrtime() whereas require('os').uptime() uses uv_uptime(). So are you referring to os.uptime() instead of process.uptime()?

@shigeki
Copy link
Contributor

shigeki commented Aug 28, 2015

I forgot the detail when I submitted netBSD patch in 3 years ago. Probably I referred FreeBSD implementation of uv_uptime().

I guess it comes from an old implementation of uptime(1) in FreeBSD and it was changed and has been using clock_gettime since 2005 as in freebsd/freebsd-src@74d3dde. We can change it in libuv now.

@yjhjstz
Copy link
Author

yjhjstz commented Aug 28, 2015

I think , we can update libuv implementation. And I was wondering why os.uptime() implementation is different from process.uptime() .

@saghul
Copy link
Member

saghul commented Aug 31, 2015

FWIW, this landed in libuv, thanks @yjhjstz!

@evanlucas
Copy link
Contributor

This was fixed by the upgrade to libuv 1.7.5. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
process Issues and PRs related to the process subsystem.
Projects
None yet
Development

No branches or pull requests

7 participants