Jesse Ruderman Assignee
	Description • 17 years ago

Loading http://stuffthathappens.com/blog/2008/03/05/simplicity/ makes Firefox crash immediately.  I got that URL from bug 427083.

The reduced testcase usually causes a crash within a few seconds, between when it says "400" and when it says "500".

If you have MallocScribble turned on, you'll see a bunch of <XXX>, where X is the Chinese character that happens to be U+5555.  This indicates that the string contents (but not the string length) are part of deleted memory.

I think what's going on here is a result of nsContentUtils::CreateContextualFragment using nsAutoTArray<nsAutoString>:

* nsAutoTArray uses memcpy when it expands.
* nsAutoString contains an internal pointer to its built-in buffer.
* Moving using memcpy causes internal pointers to point to the wrong place.

If so, this is a regression from bug 403549.  A 2007-11-12 nightly does not crash while a 2007-11-14 nightly does, which seems to support this hypothesis.

bp-6abbd978-05eb-11dd-b337-0013211cbf8a

	Jesse Ruderman Assignee
	Comment 2 • 17 years ago

Is changing nsParser::ParseFragment to take an array of nsString instead of an array of nsAutoString a reasonable solution?

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 3 • 17 years ago

Aw snap! Using nsString would work, but would churn more memory.

A better solution would be to count the length of the parent chain and call array.SetCapacity(length) before adding anything to the array.

And also add a BIG comment saying that usually putting nsAutoStrings inside an nsTArray is unsafe, but since we do the SetCapacity trick it's ok in this instance. We wouldn't want people to repeat my mistake here.

Jesse, wanna have a go at it since you already did all the work of debugging?

	Jesse Ruderman Assignee
	Comment 4 • 17 years ago

Given the number of callers of nsParser::ParseFragment, I think it would be saner to switch to nsString instead of trying to make all the callers count correctly.

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 5 • 17 years ago

Agreed. I didn't realize that we had more than one caller of this.

	Jesse Ruderman Assignee
	Comment 6 • 17 years ago

This fixes the bug for me.

	Jesse Ruderman Assignee
	Comment 7 • 17 years ago

I missed a caller in the last patch, and noticed when I did a full-tree build.

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 8 • 17 years ago

Comment on attachment 314777 [details] [diff] [review]
patch v2

Would be great if you could fix the other places in the tree that does the same thing too:

http://lxr.mozilla.org/mozilla/search?string=TArray%253CnsAutoString

	Jesse Ruderman Assignee
	Comment 9 • 17 years ago

The other places are all in gfx/thebes.  I filed bug 428447 for that.

	Mike Beltzner [:beltzner, not reading bugmail]
	Comment 10 • 17 years ago

Comment on attachment 314777 [details] [diff] [review]
patch v2

a1.9=beltzner

	Jesse Ruderman Assignee
	Comment 11 • 17 years ago

Fixed.