-
Notifications
You must be signed in to change notification settings - Fork 0
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
node 6 breaks this #1
Comments
Looks like this is sensitive to a change in v8 between 4.8.271.17 and 4.9.385.18. I was able to confirm that this breaks with node's commit 079973b96 (and works on 893835539). Updating nan/node-gyp didn't magically fix it, so I guess it's going to take a little more digging. |
This is to address #1 Some change occurred between the node repo's commits 893835539 and 079973b96 (corresponding to v8 versions 4.8.271.17 and 4.9.385.18) that makes toString() cause node to die with a `basic_string::_S_construct NULL not valid` error message. It seems that the new node.js calls the property getter with an invalid property address to query (after first calling for a toString property on instance and object). This causes the fault. It's not 100% clear what's happening, but adding a toString() method gets some functionality working. Using Object.prototype.toString.call() on the object still bombs.
This is to address #1 Some change occurred between the node repo's commits 893835539 and 079973b96 (corresponding to v8 versions 4.8.271.17 and 4.9.385.18) that makes toString() cause node to die with a `basic_string::_S_construct NULL not valid` error message. It seems that the new node.js calls the property getter with an invalid property address to query (after first calling for a toString property on instance and object). This causes the fault. It's not 100% clear what's happening, but adding a toString() method gets some functionality working. Using Object.prototype.toString.call() on the object still bombs.
I'm still not totally sure what is going on but did add a 1.0.5 is now up. |
OK, figured it out. The 1.0.6 version should no longer bomb on |
This is to address #1 Some change occurred between the node repo's commits 893835539 and 079973b96 (corresponding to v8 versions 4.8.271.17 and 4.9.385.18) that makes toString() cause node to die with a `basic_string::_S_construct NULL not valid` error message. It seems that the new node.js calls the property getter with an invalid property address to query (after first calling for a toString property on instance and object). This causes the fault. It's not 100% clear what's happening, but adding a toString() method gets some functionality working. Using Object.prototype.toString.call() on the object still bombs.
So when Node 6+ wants to do an Object.prototype.toString call, it first sends a query for a property (the GetToStringTag property, in particular). Autovivify didn't know how to deal with this query, so would bomb. This has the property getter just return if it's asked for a symbol. This lets v8 fall through to its own stringify routines, so an appropriate value is emitted ("[object AutoVivify]"). This also removes the explicit toString method as the existing fall-back one seems to work just fine. This fixes #1
First, ty for a useful module.
Next, for node 6:
$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description: Ubuntu 16.04 LTS
Release: 16.04
Codename: xenial
$ node --version
v6.1.0
$ node
The text was updated successfully, but these errors were encountered: