RESOLVED FIXED 23006
Many Loader::Host member functions are not safe to use reentrantly 
https://bugs.webkit.org/show_bug.cgi?id=23006
Summary Many Loader::Host member functions are not safe to use reentrantly
Cameron Zwarich (cpst)
Reported 2008-12-28 04:12:50 PST
Many Loader::Host member functions set m_processingResource to true when they begin processing a resource and set it to false when they are done. Thanks to JavaScript and the web inspector, almost anything can happen during the processing of a resource, including these functions being called reentrantly, which is unsafe due to this way of using m_processingResource. Since m_processingResource is used to determine whether a Host is deleted in Loader::servePendingRequests(), this is presumedly the cause behind memory corruption bugs like <rdar://problem/6216106>.
Attachments
Proposed patch (6.08 KB, patch)
2008-12-28 04:31 PST, Cameron Zwarich (cpst) 		darin: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Cameron Zwarich (cpst)
Comment 1 2008-12-28 04:13:03 PST
<rdar://problem/6216106>
Cameron Zwarich (cpst)
Comment 2 2008-12-28 04:31:52 PST
Created attachment 26275 [details] Proposed patch I am looking for a lot of review comments on this one. If anything comes to mind, don't assume that I thought about it and did it right. ;-)
Darin Adler
Comment 3 2008-12-28 11:05:52 PST
Comment on attachment 26275 [details] Proposed patch This patch is fine. We could also instead have ProcessingResource save and restore the m_processingResource boolean, which would use more stack space but less space in the object. It's really just fine as is. I'm going to say r=me I don't understand your comment "There are no occurrences of crashes caused by this bug that are reproducible by multiple people". Who ran into this? How did you diagnose it?
Cameron Zwarich (cpst)
Comment 4 2008-12-28 12:54:56 PST
I diagnosed the problem in the Radar because people were saying that it happens a lot when they are using the debugger in the inspector, which I verified causes this bad reentrancy after breaking in GDB and noticing that the JS debugger reenters the event loop. I couldn't get it to actually delete one of the Hosts that it flagged as being safe for deletion. Other people had test cases they claimed were reproducible for them at various times, but I could never make any of them work for me. Dan Bernstein was in GDB for a particular occurrence of the bug, and said that it seemed that control was being returned to Loader::Host::didFinishLoading() with a freed Loader::Host(). I just guessed that this might be the problem from all the information given.
Darin Adler
Comment 5 2008-12-28 12:59:18 PST
OK.
Cameron Zwarich (cpst)
Comment 6 2008-12-28 13:51:06 PST
Landed in r39494.
Note You need to log in before you can comment on or make changes to this bug.