-
Notifications
You must be signed in to change notification settings - Fork 3k
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
c_DOMDocument::sweep causes segfault in Joomla! JForm Test #2408
Comments
Thanks for opening this issue. Feel free (you, or anyone else) to dig deeper into this and stop it from crashing; we'd appreciate it. |
GDB Backtrace:
|
I cannot reproduce this. Please try again with latest master and let us know if you still experience it. |
This does crash in master with the jit on. |
<?php
$foo_sx = simplexml_load_string('<foo />');
$foo_dom = dom_import_simplexml($foo_sx);
$foo_dom->setAttribute('herp', 'derp');
// <foo herp="derp" /> - $foo_dom still uses the same xmlNodePtr as $foo_sx
var_dump($foo_sx->asXML()); |
Looks like a jit bug, not interp masking: <?php
function derp() {
$foo_sx = simplexml_load_string('<foo />');
$foo_dom = dom_import_simplexml($foo_sx);
$foo_dom->setAttribute('herp', 'derp');
var_dump(gettype($foo_dom->ownerDocument)); // Has a dom document
return $foo_dom;
}
var_dump(gettype(derp()->ownerDocument));
|
Our libxml memory model is fundamentally broken. We basically need an equivalent to php_libxml_node_object from php5, and both simplexml + domelement need to do their operations on this, and not own anything directly. |
@fredemmott I'm no longer seeing the behavior in your repro, any idea if this is resolved? |
My simplified example is now fine, however, the original example in the first post is still broken:
I expect we'll keep having similar bugs until DOMElement and libxml are rewritten to use a common refcounted internal libxml object (PHP5 does this), instead of the 'owner' concept |
Symptom of #4108 |
…oid deadlock. jemalloc is most likely crashed and acquired its lock if it's found in the stacktrace. Be conservative and avoid malloc again and deadlock. Part of facebook#4533 and facebook#2408.
…andler. This specifies the maximum number of seconds spent for generating a stack trace when hhvm is crashed. The default is 0 which means no timeout. This can be set to prevent from deadlocks in the backtrace handler. Part of facebook#4533 and facebook#2408.
…andler. Summary: This specifies the maximum number of seconds spent for generating a stack trace when hhvm is crashed. The default is 0 which means no timeout. This can be set to prevent from deadlocks in the backtrace handler. Part of #4533 and #2408. Closes #4828 Reviewed By: @markw65 Differential Revision: D1844818 Pulled By: @fredemmott
Summary: This diff replaces the DOMDocument based ownership model used by domdocument, and simplexml with a resource backed shared/weak pointer model roughly modeling the wrapper classes used by PHP 5. See ext_libxml.h for details on these wrappers and the PHP model for xmlNode pointers. Fixes #4108 Related to: #3899 #2408 #4084 #4086 #4128 #3096 #4542 #4526 #4688 #4754 #4767 {sync, type="parent", child="external", childrevid="35439", childdiffid="188487", parentdiffid="6996467"} Reviewed By: @bnell Differential Revision: D1927484
Code (extracted from joomlacms repository)
The text was updated successfully, but these errors were encountered: