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

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

Open
davepacheco opened this issue Feb 10, 2018 · 1 comment

Comments

@davepacheco
Copy link
Contributor

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
Copy link
Contributor

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 misterdjules@79a93c4.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants