This hangs the JS shell: gczeal(2); void <b><x/><!----></b>;
We get 100 frames deep and then hang in the loop on jsstr.c:91 Looking at this now, unless anyone else sees it instantly.
Assignee: general → crowder
Still happens on trunk. Brian, did you figure this out? I'd like to at least know whether it's a GC hazard / potential memory safety bug.
No, I didn't make much progress on it and don't have much time to spend on it at the moment. Someone else should own.
Assignee: crowder → general
I will have a look.
Assignee: general → igor
I can not reproduce the bug on Linux with the current tip. Neither pasting the command directly into the shell or via executing through a file causes any hangs.
This is sort of a GC hazard. In FoldXMLConstants we pass an unrooted |left| to js_ConcatStrings. |left| in this case is a flat string, so we set |ldep = left| and realloc its buffer. Then, we call js_NewString to create a new string to return. Because of |gczeal(2)|, we GC the string pointed to by |left| and |ldep|. Now, js_NewGCThing runs and returns the first thing on its freelist, which happens to be the string formerly known as |left| and still known as |ldep|. Here's the problem: (gdb) p str $53 = (JSString *) 0x41c598 (gdb) p ldep $54 = (JSString *) 0x41c598 The |JSPREFIX_INIT(ldep, str, ln)| on line 180 in jsstr.c is now setting both |str| and |ldep| to look like dependent strings.
Created attachment 298132 [details] [diff] [review] Possible fix This prevents |ldep| from being collected so that |str| cannot alias it.
Assignee: igor → mrbkap
Status: NEW → ASSIGNED
(In reply to comment #7) > Created an attachment (id=298132) [details] > Possible fix > > This prevents |ldep| from being collected so that |str| cannot alias it. I do not think this is a good fix. There is no GC hazard with js_ConcatString unless one supplies an unrooted left or right arguments. Which points to FoldXMLConstants, that concatenates strings without rooting them (which I should have seen long time ago). That is, FoldXMLConstants does not root accum assuming that weak rooting works. But it does not as TOK_XMLCOMMENT or TOK_XMLCDATA will displace accum from the newborn root and than later call js_ConcatStrings(cx, accuum, str) with the unrooted accum.
Right, js_ConcatStrings should be fast. Its callers are obligated to root inputs. /be
Blake's in town, this may get fixed sooner. /be
I don't know where to target this to... Guessing.
Priority: -- → P2
Target Milestone: --- → mozilla1.9beta4
Created attachment 306789 [details] [diff] [review] Better fix
Comment on attachment 306789 [details] [diff] [review] Better fix This patch fixes a GC hazard.
Comment on attachment 306789 [details] [diff] [review] Better fix We'll take this after beta4.
Target Milestone: mozilla1.9beta4 → mozilla1.9
Comment on attachment 306789 [details] [diff] [review] Better fix a1.9=beltzner
Attachment #306789 - Flags: approval1.9? → approval1.9+
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
when this bug is opened, the test should be checked in.
Flags: in-testsuite+ → in-testsuite?
You need to log in before you can comment on or make changes to this bug.