TritonDataCenter / mdb_v8

postmortem debugging for Node.js and other V8-based programs
Mozilla Public License 2.0
240 stars 18 forks source link

`findjsobjects -r` could support searching for objects it doesn't know about #110

Open davepacheco opened 6 years ago

davepacheco commented 6 years ago

Objects and arrays can refer to functions (as in the case of Node's EventEmitter), so it's useful to be able to give findjsobjects -r a closure address and have it look for objects that reference them. Today, it refuses, because it only supports looking for objects that it found during its traversal, but it doesn't record closures that it finds during its traversal (except in separate structures used for jsfunctions).

This came up while I was debugging http://smartos.org/bugview/MORAY-455. I was able to easily modify findjsobjects -r to support this, although it turned out not to help because the references I was looking for were in closure variables. Here's the diff:

diff --git a/src/mdb_v8.c b/src/mdb_v8.c
index a81dfa3..1b58797 100644
--- a/src/mdb_v8.c
+++ b/src/mdb_v8.c
@@ -5724,12 +5724,13 @@ dcmd_findjsobjects(uintptr_t addr,
                 */
                inst = findjsobjects_instance(fjs, addr, &head);

-               if (inst == NULL) {
-                       mdb_warn("%p is not a valid object\n", addr);
-                       return (DCMD_ERR);
-               }
-
                if (!references && !fjs->fjs_marking) {
+                       if (inst == NULL) {
+                               mdb_warn("%p is not a valid object\n", addr);
+                               return (DCMD_ERR);
+                       }
+
+                       assert(head != NULL);
                        for (inst = head; inst != NULL; inst = inst->fjsi_next)
                                mdb_printf("%p\n", inst->fjsi_addr);

@@ -5737,8 +5738,14 @@ dcmd_findjsobjects(uintptr_t addr,
                }

                if (!listlike) {
-                       findjsobjects_referent(fjs, inst->fjsi_addr);
+                       findjsobjects_referent(fjs, addr);
                } else {
+                       if (inst == NULL) {
+                               mdb_warn("%p is not a valid object\n", addr);
+                               return (DCMD_ERR);
+                       }
+
+                       assert(head != NULL);
                        for (inst = head; inst != NULL; inst = inst->fjsi_next)
                                findjsobjects_referent(fjs, inst->fjsi_addr);
                }

We should integrate this, since it can be useful.

misterdjules commented 6 years ago

it turned out not to help because the references I was looking for were in closure variables

I thought I'd mention a very rough draft of changes that make findjsobjects -r consider closure variables when looking for referents. It's at https://github.com/misterdjules/mdb_v8/commit/79a93c474a3e7ba1d867ce03315be2a469a88d3a.

I'm not necessarily proud of this code, and it's implemented using a brute force algorithm that goes through all function objects and their closures to find potential referents, but it already helped me root cause some memory leaks that I wouldn't have been to find otherwise.