Additional information for libuv handles #73

richardlau · 2017-03-08T14:49:51Z

Adds details for libuv handle types.

e.g.

================================================================================
==== Node.js libuv Handle Summary ==============================================

(Flags: R=Ref, A=Active)
Flags  Type      Address             Details
[-A]   async     0x00000000019d7260
[--]   check     0x0000000002be01a0
[R-]   idle      0x0000000002be0218
[--]   prepare   0x0000000002be0308
[--]   check     0x0000000002be0380
[--]   idle      0x0000000002be0290
[R-]   timer     0x0000000002be0418  repeat: 0
[-A]   async     0x00007f1f823b4ca0
[RA]   fs_event  0x0000000002bdd6e8  filename: /home/users/riclau/sandbox/github/nodereport/test/test-api-uvhandles.js
[RA]   fs_poll   0x0000000002bdd510  filename: /home/users/riclau/sandbox/github/nodereport/test/test-api-uvhandles.js
[RA]   tcp       0x0000000002bf8dc0  :::42241 (not connected) send buffer size: 16384 recv buffer size: 87380 file descriptor: 11 write queue size 0  readable  writable
[RA]   tcp       0x0000000002bfb080  localhost:49618 connected to localhost:42241 send buffer size: 2626560 recv buffer size: 1061296 file descriptor: 12 write queue size 0  readable  writable
[RA]   tcp       0x0000000002bfb220  localhost:42241 connected to localhost:49618 send buffer size: 2626560 recv buffer size: 1061488 file descriptor: 13 write queue size 0  readable  writable
[-A]   timer     0x0000000002bf9ee8  repeat: 0
[R-]   tty       0x0000000002beff90  width: 111 height: 65 file descriptor: 14 write queue size 0  writable
[-A]   signal    0x0000000002bf0118  signum: 28(SIGWINCH)
[R-]   tty       0x0000000002bf0260  width: 111 height: 65 file descriptor: 15 write queue size 0  writable

Suggestions welcome on formatting. For the moment I've kept the first three columns as they were (originally based on uv_print_all_handles but rewritten in #48). Perhaps we can turn readable/writable into flags?

cc @hhellyer

richardlau · 2017-03-08T15:17:43Z

Node.js 8 nightly CI: https://ci.nodejs.org/view/post-mortem/job/nodereport-continuous-integration/127/

sam-github · 2017-03-08T15:20:17Z

signum: 28(SIGWINCH) -> signum: 28 (SIGWINCH)

hhellyer · 2017-03-09T10:59:44Z

Looking at the libuv api’s uv_tcp_getsockname has equivalents uv_udp_getsockname and uv_pipe_getsockname so we should probably add cases to support those.

uv_process_t could probably display pid and command line options from http://docs.libuv.org/en/v1.x/process.html

Could the code for extending buffers in the UV_FS_EVENT and UV_FS_POLL cases be de-duplicated? (If you add support for the other getsockname calls it might be worth pulling that code out as functions to make the case statement smaller too.)

Adding read/write flags sounds like a better approach but it adds columns that only apply to some types. I’m not sure how best we can make the output a bit easier to read but I think that’s along the right lines. Could we group handles together by type or common fields (e.g. tcp/udp/pipes together) so we can add some extra column headings instead of the “fieldname: value” format? I don’t want to do too much processing in node-report, it’s supposed to be quick but it’s also supposed to be human readable.

hhellyer · 2017-03-10T10:49:53Z

src/node_report.cc

+      free(buffer); \
+    } \
+  } \
+  while (0)


It would be better to pull out the filename writing code as a function. You can pass the handle with uv_any_handle* handle and check if it's a poll or event where it needs to using handle->handle.type.

hhellyer · 2017-03-10T10:52:38Z

src/node_report.cc

-    case UV_TCP: type = "tcp"; break;
-    case UV_TIMER: type = "timer"; break;
-    case UV_TTY: type = "tty"; break;
+    case UV_TCP: {


Should probably pull all this out as a function as well and support UDP and PIPE handles with uv_udp_getsockname and uv_pipe_getsockname there too.

hhellyer · 2017-03-10T11:44:25Z

src/node_report.cc

-  out << "\nFlags  Type      Address\n";
+  out << std::left << std::setw(7) << "Flags" << std::setw(10) << "Type"
+      << std::setw(4 + 2 * sizeof(void*)) << "Address" << "Details"
+      << std::endl;


I've been caught out by setting formatting flags before, they are set on the stream so any other users will be affected by them later on. (It's a problem if we are writing to stdout or stderr!)

Using them is fine but I think we should probably call:
std::ios_base::fmtflags format = out.flags();
to save them at the start of WriteNodeReport and:
out.flags(format);
so we leave the streams as we found them. (I think that will reset things like fill characters but we should probably test it to be sure.)

That doesn't reset fill, but copyfmt does so I've used that instead.

Fishrock123 · 2017-03-13T18:54:02Z

Is it not possible to get the timer duration from timers?

Also, maybe comma-separate fields in the "Details"?

richardlau · 2017-03-13T19:59:16Z

Is it not possible to get the timer duration from timers?

Doesn't look like it is: http://docs.libuv.org/en/v1.x/timer.html

Refs: nodejs/node-report#73

Fishrock123 · 2017-03-16T16:30:58Z

@richardlau can you try just using handle->timeout? e.g. libuv/libuv#1255 (comment)

hhellyer · 2017-03-16T16:53:49Z

@Fishrock123 - That looks good but might not help for older levels of libuv. Would an alternative be to update the libuv docs to say handle->timeout is a public field that can be read?
It's declared in UV_TIMER_PRIVATE_FIELDS in uv.h but looking at the other public fields on other handles a lot of them look like they are intended to be accessed read only from outside libuv.

Fishrock123 · 2017-03-16T20:59:49Z

@hhellyer you're right about the older versions thing, would an #ifdef or something be able to handle that though?

Refs: nodejs/node-report#73

richardlau · 2017-03-24T18:23:32Z

Factored out reporting of paths for fs events/poll and endpoints for tcp and udp. uv_pipe_getsockname/uv_pipe_getpeername take different argument types compared to the tcp/udp equivalents so I'm undecided as to whether to handle pipes in the same function.

TODO:

richardlau · 2017-05-05T17:15:07Z

Implemented todos in previous comment and rebased onto current master.

PTAL.

richardlau · 2017-05-05T17:18:53Z

v8.0.0-nightly CI: https://ci.nodejs.org/view/post-mortem/job/nodereport-continuous-integration/157/

richardlau · 2017-05-05T17:23:56Z

Failures on Windows:
e.g. https://ci.nodejs.org/view/post-mortem/job/nodereport-continuous-integration/157/MACHINE=win2012r2/console

..\src\node_report.cc(623): error C2039: 'timeout': is not a member of 'uv_timer_s' [C:\workspace\nodereport-continuous-integration\MACHINE\win2012r2\node-report\build\api.vcxproj]
  C:\Users\dev\.node-gyp\8.0.0-nightly201705043f48ab3042\include\node\uv.h(778): note: see declaration of 'uv_timer_s'

richardlau · 2017-05-05T17:39:31Z

Okay, changed to use timer.due on Windows instead of timer.timeout. Trying again.

v8.0.0-nightly CI: https://ci.nodejs.org/view/post-mortem/job/nodereport-continuous-integration/158/

richardlau · 2017-05-05T17:44:07Z

TODO (probably Monday):

Check ((timer.due|timer.timeout) - uv_now(handle->timer.loop)) > 0 (getting large values if the timer has expired)

hhellyer · 2017-05-09T15:39:03Z

Looks good. I think timer values are unsigned so I'm wondering if there's some implementation details in Node.js that causes them to be very large deliberately. (But to be honest I'm not really sure!)

rnchamberlain · 2017-05-10T10:11:41Z

src/node_report.cc

+
+  if (h->type == UV_TCP || h->type == UV_NAMED_PIPE || h->type == UV_TTY) {
+
+    data << ", write queue size "


Minor: could you add the usual colon after 'write queue size' to separate the name from the value, for consistency

richardlau · 2017-05-10T18:40:00Z

Added the colon after write queue size and tidied up the reporting of timers. I'm not sure repeat is useful -- It always seems to be 0 even for setInterval() (Node.js' way of creating repeating timers). I've left it in for now.

I'll start some CI runs. Let me know if you want me to squash the fixup commits, or if you'll do that when landing.

richardlau · 2017-05-10T18:47:06Z

Node.js v4 CI: https://ci.nodejs.org/view/post-mortem/job/nodereport-continuous-integration/160/
Node.js v6 CI: https://ci.nodejs.org/view/post-mortem/job/nodereport-continuous-integration/161/

Seems to be some missing v8.0.0-nightly builds: https://ci.nodejs.org/view/post-mortem/job/nodereport-continuous-integration/159/

rnchamberlain · 2017-05-11T09:14:05Z

@richardlau we squash commits when landing, so no need for you to do it

Agree the v8 CI build failures are just missing Node build .zip files
LGTM

hhellyer · 2017-05-11T10:15:53Z

@richardlau - I'm fine with leaving repeat in, Node.js could change it's implementation and native npms can access libuv directly and I think could (in theory) put their own timers on the event loop.

rnchamberlain · 2017-05-12T09:33:58Z

Landing this shortly

rnchamberlain · 2017-05-12T10:23:24Z

@richardlau sorry, could you rebase this please, there is a clash with #82 that landed recently

Also I think there is a test failure in test/test-api-uvhandles.js on one platform:
https://ci.nodejs.org/view/post-mortem/job/nodereport-continuous-integration/161/MACHINE=aix61-ppc64/consoleText

richardlau · 2017-05-18T18:47:07Z

@rnchamberlain I've rebased onto the current master, but now I get a compilation failure with Node.js 4.8.3:

Node.js 4.8.3 compilation failure

-bash-4.2$ npm install

> node-report@2.1.2 install /home/users/riclau/sandbox/github/nodereport
> node-gyp rebuild

make: Entering directory `/home/users/riclau/sandbox/github/nodereport/build'
  CXX(target) Release/obj.target/api/src/node_report.o
../src/node_report.cc: In function ‘void nodereport::PrintJavaScriptErrorStack(std::ostream&, v8::Isolate*, v8::MaybeLocal<v8::Value>)’:
../src/node_report.cc:1008:88: error: no matching function for call to ‘v8::Exception::CreateMessage(v8::Isolate*&, v8::Local<v8::Value>)’
   Local<Message> message = v8::Exception::CreateMessage(isolate, error.ToLocalChecked());
                                                                                        ^
../src/node_report.cc:1008:88: note: candidate is:
In file included from /home/users/riclau/.node-gyp/4.8.3/include/node/node.h:42:0,
                 from ../node_modules/nan/nan.h:47,
                 from ../src/node_report.h:4,
                 from ../src/node_report.cc:1:
/home/users/riclau/.node-gyp/4.8.3/include/node/v8.h:4874:25: note: static v8::Local<v8::Message> v8::Exception::CreateMessage(v8::Local<v8::Value>)
   static Local<Message> CreateMessage(Local<Value> exception);
                         ^
/home/users/riclau/.node-gyp/4.8.3/include/node/v8.h:4874:25: note:   candidate expects 1 argument, 2 provided
make: *** [Release/obj.target/api/src/node_report.o] Error 1
make: Leaving directory `/home/users/riclau/sandbox/github/nodereport/build'

richardlau · 2017-05-18T18:56:50Z

Looking into AIX failure,

not ok 3 - Checking no messages on stderr
      ---
      found: >

        Writing Node.js report to file: node-report.20170511.015832.5439724.001.txt

        Node.js report completed


        Uncaught exception at:

        FSWatcher.start (fs.js:1445:5)

        Object.fs.watch (fs.js:1470:11)

        Object.<anonymous>
        (/home/iojs/build/workspace/nodereport-continuous-integration/MACHINE/aix61-ppc64/node-report/test/test-api-uvhandles.js:15:22)

        Module._compile (module.js:570:32)

        Object.Module._extensions..js (module.js:579:10)

        Module.load (module.js:487:32)

        tryModuleLoad (module.js:446:12)

        Function.Module._load (module.js:438:3)

        Module.runMain (module.js:604:10)

        run (bootstrap_node.js:390:7)

        startup (bootstrap_node.js:150:9)

        bootstrap_node.js:505:3
      wanted: ''

hhellyer · 2017-05-19T07:59:09Z

@richardlau - The compilation failure with 4.8.3 is in existing code added for PR #82 so I've raised a separate issue #87.

hhellyer · 2017-05-19T08:10:18Z

The line that fails on AIX is using fs.watch, this has limitations on AIX:
https://nodejs.org/dist/latest-v6.x/docs/api/fs.html#fs_availability
we should probably skip the fs.watch related test on AIX as it likely to pass/fail depending on the type of file system it's running on.

(fs.watchFile should still work, fs.watch is just more efficient.)

richardlau · 2017-05-25T17:21:05Z

Rebased onto current master to pick up the 4.8.3 compilation failure fix and updated testcase to skip the fs_event handle check if fs.watch() failed for any reason.

Node.js v4.x CI: https://ci.nodejs.org/view/post-mortem/job/nodereport-continuous-integration/168/
Node.js v8.0.0-nightly CI: https://ci.nodejs.org/view/post-mortem/job/nodereport-continuous-integration/169/

rnchamberlain · 2017-05-26T09:00:56Z

Great, thanks @richardlau, landing this shortly

PR-URL: #73 Reviewed-By: Howard Hellyer <hhellyer@uk.ibm.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Richard Chamberlain <richard_chamberlain@uk.ibm.com>

rnchamberlain · 2017-05-26T11:50:23Z

Landed as c7056c5, many thanks @richardlau @hhellyer and reviewers

richardlau · 2017-05-26T18:11:24Z

Forgot the original AIX failure was on Node.js v6 (fs.watch() was reenabled a later libuv included in the Node.js v8.0.0-nightlies).

Node.js v6.x CI: https://ci.nodejs.org/view/post-mortem/job/nodereport-continuous-integration/180/

And for posterity: https://ci.nodejs.org/view/post-mortem/job/nodereport-continuous-integration/180/MACHINE=aix61-ppc64/console

Passing test on AIX (fs.watch() skipped)

# Subtest: test/test-api-uvhandles.js
    1..14
    ok 1 - Process should exit with expected exit code
    ok 2 - Process should exit cleanly
    ok 3 - Checking no messages on stderr
    ok 4 - Checking no report files were written
    not ok 5 - Checking fs_event uv handle # SKIP fs.watch() unavailable
    ok 6 - Checking fs_poll uv handle
    ok 7 - Checking process uv handle
    ok 8 - Checking timer uv handle
    ok 9 - Checking pipe uv handle
    ok 10 - Checking listening socket tcp uv handle
    ok 11 - Checking inbound connection tcp uv handle
    ok 12 - Checking outbound connection tcp uv handle
    ok 13 - Checking udp uv handle
    # Subtest: Validating report content
        1..19
        ok 1 - Checking report contains Node Report section
        ok 2 - Checking report contains JavaScript Stack Trace section
        ok 3 - Checking report contains JavaScript Heap section
        ok 4 - Checking report contains System Information section
        ok 5 - Node Report header section contains expected process ID
        ok 6 - Node Report header section contains expected Node.js version
        ok 7 - Checking report contains expected command line
        ok 8 - Node Report header section contains expected http_parser version
        ok 9 - Node Report header section contains expected v8 version
        ok 10 - Node Report header section contains expected uv version
        ok 11 - Node Report header section contains expected zlib version
        ok 12 - Node Report header section contains expected ares version
        ok 13 - Node Report header section contains expected icu version
        ok 14 - Node Report header section contains expected modules version
        ok 15 - Node Report header section contains expected openssl version
        ok 16 - Node Report header section contains expected Node Report version
        ok 17 - Checking machine name in report header section contains os.hostname()
        ok 18 - Checking OS version
        ok 19 - System Information section contains node-report library.
    ok 14 - Validating report content # time=10.701ms
    # time=499.158ms
ok 8 - test/test-api-uvhandles.js # time=927.647ms

Refs: nodejs/node-report#73

Co-authored-by: Jeremiah Senkpiel <fishrock123@rocketmail.com> Refs: nodejs/node-report#73 Refs: #1255 Fixes: #2950 PR-URL: #2951 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

richardlau force-pushed the uv_handles branch from cbf4a10 to 4815c8a Compare March 9, 2017 20:33

hhellyer suggested changes Mar 10, 2017

View reviewed changes

hhellyer reviewed Mar 10, 2017

View reviewed changes

Fishrock123 added a commit to Fishrock123/libuv that referenced this pull request Mar 16, 2017

unix,win: add uv_timer_get_timeout()

916d48c

Refs: nodejs/node-report#73

Fishrock123 mentioned this pull request Mar 16, 2017

unix,win: add uv_timer_get_timeout() libuv/libuv#1255

Closed

Fishrock123 added a commit to Fishrock123/libuv that referenced this pull request Mar 21, 2017

unix,win: add uv_timer_get_timeout()

1d878d2

Refs: nodejs/node-report#73

Fishrock123 added a commit to Fishrock123/libuv that referenced this pull request Mar 22, 2017

unix,win: add uv_timer_get_timeout()

37fff3a

Refs: nodejs/node-report#73

richardlau force-pushed the uv_handles branch from 4815c8a to 0793625 Compare March 24, 2017 18:17

richardlau force-pushed the uv_handles branch from d3b8a03 to 324eeae Compare May 5, 2017 17:11

rnchamberlain reviewed May 10, 2017

View reviewed changes

hhellyer approved these changes May 11, 2017

View reviewed changes

richardlau force-pushed the uv_handles branch from 218cc99 to 076c729 Compare May 18, 2017 18:12

Additional information for libuv handles

39b9c8d

richardlau force-pushed the uv_handles branch from 076c729 to 39b9c8d Compare May 25, 2017 17:08

rnchamberlain closed this May 26, 2017

ulrikstrid pushed a commit to ulrikstrid/libuv that referenced this pull request Aug 6, 2020

unix,win: add uv_timer_get_timeout()

8584d3d

Refs: nodejs/node-report#73

ulrikstrid pushed a commit to ulrikstrid/libuv that referenced this pull request Aug 6, 2020

unix,win: add uv_timer_get_timeout()

3fbf000

Refs: nodejs/node-report#73

ulrikstrid pushed a commit to ulrikstrid/libuv that referenced this pull request Aug 6, 2020

unix,win: add uv_timer_get_timeout()

b1a2e8c

Refs: nodejs/node-report#73

ulrikstrid mentioned this pull request Aug 6, 2020

Add uv timer get due in libuv/libuv#2951

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Additional information for libuv handles #73

Additional information for libuv handles #73

richardlau commented Mar 8, 2017

richardlau commented Mar 8, 2017

sam-github commented Mar 8, 2017

hhellyer commented Mar 9, 2017

hhellyer Mar 10, 2017

hhellyer Mar 10, 2017

hhellyer Mar 10, 2017

richardlau Mar 28, 2017 •

edited

Loading

Fishrock123 commented Mar 13, 2017

richardlau commented Mar 13, 2017

Fishrock123 commented Mar 16, 2017

hhellyer commented Mar 16, 2017

Fishrock123 commented Mar 16, 2017

richardlau commented Mar 24, 2017 •

edited

Loading

richardlau commented May 5, 2017

richardlau commented May 5, 2017

richardlau commented May 5, 2017

richardlau commented May 5, 2017

richardlau commented May 5, 2017 •

edited

Loading

hhellyer commented May 9, 2017

rnchamberlain May 10, 2017

richardlau commented May 10, 2017

richardlau commented May 10, 2017

rnchamberlain commented May 11, 2017

hhellyer commented May 11, 2017

rnchamberlain commented May 12, 2017

rnchamberlain commented May 12, 2017 •

edited

Loading

richardlau commented May 18, 2017

richardlau commented May 18, 2017

hhellyer commented May 19, 2017

hhellyer commented May 19, 2017

richardlau commented May 25, 2017

rnchamberlain commented May 26, 2017

rnchamberlain commented May 26, 2017

richardlau commented May 26, 2017


		if (h->type == UV_TCP \|\| h->type == UV_NAMED_PIPE \|\| h->type == UV_TTY) {

		data << ", write queue size "

Additional information for libuv handles #73

Additional information for libuv handles #73

Conversation

richardlau commented Mar 8, 2017

richardlau commented Mar 8, 2017

sam-github commented Mar 8, 2017

hhellyer commented Mar 9, 2017

hhellyer Mar 10, 2017

Choose a reason for hiding this comment

hhellyer Mar 10, 2017

Choose a reason for hiding this comment

hhellyer Mar 10, 2017

Choose a reason for hiding this comment

richardlau Mar 28, 2017 • edited Loading

Choose a reason for hiding this comment

Fishrock123 commented Mar 13, 2017

richardlau commented Mar 13, 2017

Fishrock123 commented Mar 16, 2017

hhellyer commented Mar 16, 2017

Fishrock123 commented Mar 16, 2017

richardlau commented Mar 24, 2017 • edited Loading

richardlau commented May 5, 2017

richardlau commented May 5, 2017

richardlau commented May 5, 2017

richardlau commented May 5, 2017

richardlau commented May 5, 2017 • edited Loading

hhellyer commented May 9, 2017

rnchamberlain May 10, 2017

Choose a reason for hiding this comment

richardlau commented May 10, 2017

richardlau commented May 10, 2017

rnchamberlain commented May 11, 2017

hhellyer commented May 11, 2017

rnchamberlain commented May 12, 2017

rnchamberlain commented May 12, 2017 • edited Loading

richardlau commented May 18, 2017

richardlau commented May 18, 2017

hhellyer commented May 19, 2017

hhellyer commented May 19, 2017

richardlau commented May 25, 2017

rnchamberlain commented May 26, 2017

rnchamberlain commented May 26, 2017

richardlau commented May 26, 2017

richardlau Mar 28, 2017 •

edited

Loading

richardlau commented Mar 24, 2017 •

edited

Loading

richardlau commented May 5, 2017 •

edited

Loading

rnchamberlain commented May 12, 2017 •

edited

Loading