<div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">[resend with nicer formatting]</div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">

<br></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">I just finished fixing <a href="http://crbug.com/75604" target="_blank" style="color: rgb(17, 85, 204); ">http://crbug.com/75604</a> aka <a href="https://bugs.webkit.org/show_bug.cgi?id=71122" target="_blank" style="color: rgb(17, 85, 204); ">https://bugs.webkit.org/show_bug.cgi?id=71122</a> , and it was a long process, and pretty involved, and had a lot of cycles of change, wait for crash data, etc...  So here's my postmortum:</div>

<div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); "><br></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">

TL;DR No matter how sure you are you fixed a crashing bug, keep reading crash reports to be sure.</div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">

<br></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">TL;DR2 Practice release crash dump debugging regularly.  Read dissassembled code and never trust symbols.</div>

<div class="im" style="color: rgb(80, 0, 80); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); "><div><br></div><div>TL;DR3 When in doubt, add more state to enhance crash dumps.</div>

<div><br></div></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">TL;DR4 WebKit can save stacks in memory now.  <a href="mailto:gavinp@webkit.org" target="_blank" style="color: rgb(17, 85, 204); ">gavinp@webkit.org</a> can help you use this.</div>

<div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); "><br></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">

<br></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">WHAT WAS HAPPENING</div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">

<br></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">The ScriptRunner was crashing, and we were getting a lot of uploads. We couldn't reproduce it by going to the same URIs, but it was definitely happening a lot in the wild.  It didn't seem like other WebKit ports were seeing this bug either, which was interesting.</div>

<div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); "><br></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">

<br></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">WHAT WE DID</div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">

<br></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">1. The crash in ScriptRunner was happening in  ScriptRunner::timerFired(), and it looked like a CachedScript in the list of runnable scripts was NULL.  How did that happen?  So I added a CRASH() to ScriptRunner::queueScriptForExecution().  This meant we would crash before setting up the Async event, and maybe get a more interesting stack on our crash dumps?</div>

<div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); "><a href="https://bugs.webkit.org/show_bug.cgi?id=65563" target="_blank" style="color: rgb(17, 85, 204); ">https://bugs.webkit.org/show_bug.cgi?id=65563</a></div>

<div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); "><br></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">

2. From this, we learned a few things.  Crash dumps told us we definitely had a broken ScriptElement, and every stack showed a network error was in progress.  The CachedScript in the ScriptElement was bad, and we couldn't figure out why.  So, we added a hidden state machine to ScriptElement to track down how it was being zeroed.  Was it double notification, or was the load continuing after being stopped?  Crash dumps would tell.</div>

<div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); "><a href="https://bugs.webkit.org/show_bug.cgi?id=66939" target="_blank" style="color: rgb(17, 85, 204); ">https://bugs.webkit.org/show_bug.cgi?id=66939</a></div>

<div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); "><br></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">

3. It was double notification.  Nobody could figure out how on earth double notification happened.  Boy, I'd sure like to know the stack at the time of the first ScriptElement::notifyFinished() call...  So we added a new stack saving facility to WebKit, and used it here to save a stack inside the ScriptElement on every call to ScriptElement::notifyFinished().  Mosquito, meet our sledgehammer.</div>

<div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); "><a href="https://bugs.webkit.org/show_bug.cgi?id=69453" target="_blank" style="color: rgb(17, 85, 204); ">https://bugs.webkit.org/show_bug.cgi?id=69453</a></div>

<div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); "><br></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">

4. Oh, so the earlier stack was data received, but with a response code >= 400?  How interesting...  Finally I could reproduce the bug, and introduce a fix.</div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">

<a href="https://bugs.webkit.org/show_bug.cgi?id=71122" target="_blank" style="color: rgb(17, 85, 204); ">https://bugs.webkit.org/show_bug.cgi?id=71122</a></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">

<br></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); "><br></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">

WHAT WORKED</div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); "><br></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">

1. Windows minidumps.  In Chrome, our dev and canary channels have enabled a very useful option that gives you some heap.  In walking the stack on Windows when crashing, any stack element interpretable as a pointer gets the 1024byte range [p-256,p+768] saved into the dump. This feature makes debugging from dev or canary windows minidumps superior to anything else.  Learn how to use windb(a)g.</div>

<div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); "><br></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">

2. Progressive instrumentation.  As usual, once I had a reproduction a fix was pretty easy.  But, while I didn't have a reproduction, it was useful to add release assertions and watch crash dumps, to progressively zone in on this bug.</div>

<div class="im" style="color: rgb(80, 0, 80); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); "><div><br></div><div>3. Adding new facilities to WebKit.  You want stacks saved in memory on every platform?  OK.</div>

<div><a href="https://bugs.webkit.org/show_bug.cgi?id=69018" target="_blank" style="color: rgb(17, 85, 204); ">https://bugs.webkit.org/show_bug.cgi?id=69018</a></div><div><br></div><div><br></div><div>WHAT DIDN'T WORK</div>

<div><br></div></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">1. Debuggers.  Hackers who should have known better looked at release minidumps and symbols, and couldn't understand the variables.  For a good example of this mistake from me, see <a href="http://code.google.com/p/chromium/issues/detail?id=75604#c21" target="_blank" style="color: rgb(17, 85, 204); ">http://code.google.com/p/chromium/issues/detail?id=75604#c21</a> .  Want to see someone even smarter make the same mistake?</div>

<div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); "><a href="http://code.google.com/p/chromium/issues/detail?id=75604#c43" target="_blank" style="color: rgb(17, 85, 204); ">http://code.google.com/p/chromium/issues/detail?id=75604#c43</a></div>

<div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); "><br></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">

The ScriptElement wasn't bogus, but the symbols data was.  The symbols indicated ECX had this, when this was actually in EAX at the time, or [ESP].  Don't trust symbols in release builds; instead dissassemble the function, and figure out where the data is.  The symbols probably said ECX because at the point of first dereference or maybe at call time, in that scope, this was probably in ECX.  But in optimised code, that doesn't have much to do with anything anywhere else in the code. Read the dissassembly.</div>

<div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); "><br></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">

2. Reading code.  I spent far, far too much time doing this.  You know what, usually this works for me: I read code, consider the stack, have an AHA! moment, and fix my bug.  Too bad this bug was beyond my cognitive horizon, and reading code was just a great way to waste time.</div>

<div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); "><br></div><div style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); ">

3. Not watching crashes.  See comments 30 and 31 in the bug: <a href="http://code.google.com/p/chromium/issues/detail?id=75604#c30" target="_blank" style="color: rgb(17, 85, 204); ">http://code.google.com/p/chromium/issues/detail?id=75604#c30</a> .  We should not have had confidence the problem was solved; since, hey, it wasn't.</div>

<div class="HOEnZb TFaZ0b" style="color: rgb(34, 34, 34); font-family: arial, sans-serif; font-size: 13px; background-color: rgba(255, 255, 255, 0.917969); "><div class="im" style="color: rgb(80, 0, 80); "><div><br></div>

<div>SPECIAL THANKS TO</div><div><br></div><div><a href="mailto:ap@webkit.org" target="_blank" style="color: rgb(17, 85, 204); ">ap@webkit.org</a> for encouraging me to instrument where it helps.</div><div><a href="mailto:cbentzel@chromium.org" target="_blank" style="color: rgb(17, 85, 204); ">cbentzel@chromium.org</a> for putting up with my yak shaving.</div>

<div><a href="mailto:eroman@chromium.org" target="_blank" style="color: rgb(17, 85, 204); ">eroman@chromium.org</a> for showing me how to use windb(a)g.</div><div><a href="mailto:japhet@chromium.org" target="_blank" style="color: rgb(17, 85, 204); ">japhet@chromium.org</a> for never giving up hope.</div>

</div></div><br><div class="gmail_quote">On 2 November 2011 12:06, Gavin Peters (蓋文彼德斯) <span dir="ltr"><<a href="mailto:gavinp@chromium.org">gavinp@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

I just finished fixing <a href="http://crbug.com/75604" target="_blank">http://crbug.com/75604</a> aka<br>
<a href="https://bugs.webkit.org/show_bug.cgi?id=71122" target="_blank">https://bugs.webkit.org/show_bug.cgi?id=71122</a> , and it was a long<br>
process, and pretty involved, and had a lot of cycles of change, wait<br>
for crash data, etc...  So here's my postmortum:<br>
<br>
TL;DR No matter how sure you are you fixed a crashing bug, keep<br>
reading crash reports to be sure.<br>
<br>
TL;DR2 Practice release crash dump debugging regularly.  Read<br>
dissassembled code and never trust symbols.<br>
<br>
TL;DR3 When in doubt, add more state to enhance crash dumps.<br>
<br>
TL;DR4 WebKit can save stacks in memory now.  <a href="mailto:gavinp@webkit.org">gavinp@webkit.org</a> can<br>
help you use this.<br>
<br>
<br>
WHAT WAS HAPPENING<br>
<br>
The ScriptRunner was crashing, and we were getting a lot of uploads.<br>
We couldn't reproduce it by going to the same URIs, but it was<br>
definitely happening a lot in the wild.  It didn't seem like other<br>
WebKit ports were seeing this bug either, which was interesting.<br>
<br>
<br>
WHAT WE DID<br>
<br>
1. The crash in ScriptRunner was happening in<br>
ScriptRunner::timerFired(), and it looked like a CachedScript in the<br>
list of runnable scripts was NULL.  How did that happen?  So I added a<br>
CRASH() to ScriptRunner::queueScriptForExecution().  This meant we<br>
would crash before setting up the Async event, and maybe get a more<br>
interesting stack on our crash dumps?<br>
<a href="https://bugs.webkit.org/show_bug.cgi?id=65563" target="_blank">https://bugs.webkit.org/show_bug.cgi?id=65563</a><br>
<br>
2. From this, we learned a few things.  Crash dumps told us we<br>
definitely had a broken ScriptElement, and every stack showed a<br>
network error was in progress.  The CachedScript in the ScriptElement<br>
was bad, and we couldn't figure out why.  So, we added a hidden state<br>
machine to ScriptElement to track down how it was being zeroed.  Was<br>
it double notification, or was the load continuing after being<br>
stopped?  Crash dumps would tell.<br>
<a href="https://bugs.webkit.org/show_bug.cgi?id=66939" target="_blank">https://bugs.webkit.org/show_bug.cgi?id=66939</a><br>
<br>
3. It was double notification.  Nobody could figure out how on earth<br>
double notification happened.  Boy, I'd sure like to know the stack at<br>
the time of the first ScriptElement::notifyFinished() call...  So we<br>
added a new stack saving facility to WebKit, and used it here to save<br>
a stack inside the ScriptElement on every call to<br>
ScriptElement::notifyFinished().  Mosquito, meet our sledgehammer.<br>
<a href="https://bugs.webkit.org/show_bug.cgi?id=69453" target="_blank">https://bugs.webkit.org/show_bug.cgi?id=69453</a><br>
<br>
4. Oh, so the earlier stack was data received, but with a response<br>
code >= 400?  How interesting...  Finally I could reproduce the bug,<br>
and introduce a fix.<br>
<a href="https://bugs.webkit.org/show_bug.cgi?id=71122" target="_blank">https://bugs.webkit.org/show_bug.cgi?id=71122</a><br>
<br>
<br>
WHAT WORKED<br>
<br>
1. Windows minidumps.  In Chrome, our dev and canary channels have<br>
enabled a very useful option that gives you some heap.  In walking the<br>
stack on Windows when crashing, any stack element interpretable as a<br>
pointer gets the 1024byte range [p-256,p+768] saved into the dump.<br>
This feature makes debugging from dev or canary windows minidumps<br>
superior to anything else.  Learn how to use windb(a)g.<br>
<br>
2. Progressive instrumentation.  As usual, once I had a reproduction a<br>
fix was pretty easy.  But, while I didn't have a reproduction, it was<br>
useful to add release assertions and watch crash dumps, to<br>
progressively zone in on this bug.<br>
<br>
3. Adding new facilities to WebKit.  You want stacks saved in memory<br>
on every platform?  OK.<br>
<a href="https://bugs.webkit.org/show_bug.cgi?id=69018" target="_blank">https://bugs.webkit.org/show_bug.cgi?id=69018</a><br>
<br>
<br>
WHAT DIDN'T WORK<br>
<br>
1. Debuggers.  Hackers who should have known better looked at release<br>
minidumps and symbols, and couldn't understand the variables.  For a<br>
good example of this mistake from me, see<br>
<a href="http://code.google.com/p/chromium/issues/detail?id=75604#c21" target="_blank">http://code.google.com/p/chromium/issues/detail?id=75604#c21</a> .  Want<br>
to see someone even smarter make the same mistake?<br>
<a href="http://code.google.com/p/chromium/issues/detail?id=75604#c43" target="_blank">http://code.google.com/p/chromium/issues/detail?id=75604#c43</a><br>
<br>
The ScriptElement wasn't bogus, but the symbols data was.  The symbols<br>
indicated ECX had this, when this was actually in EAX at the time, or<br>
[ESP].  Don't trust symbols in release builds; instead dissassemble<br>
the function, and figure out where the data is.  The symbols probably<br>
said ECX because at the point of first dereference or maybe at call<br>
time, in that scope, this was probably in ECX.  But in optimised code,<br>
that doesn't have much to do with anything anywhere else in the code.<br>
Read the dissassembly.<br>
<br>
2. Reading code.  I spent far, far too much time doing this.  You know<br>
what, usually this works for me: I read code, consider the stack, have<br>
an AHA! moment, and fix my bug.  Too bad this bug was beyond my<br>
cognitive horizon, and reading code was just a great way to waste<br>
time.<br>
<br>
3. Not watching crashes.  See comments 30 and 31 in the bug:<br>
<a href="http://code.google.com/p/chromium/issues/detail?id=75604#c30" target="_blank">http://code.google.com/p/chromium/issues/detail?id=75604#c30</a> .  We<br>
should not have had confidence the problem was solved; since, hey, it<br>
wasn't.<br>
<br>
<br>
SPECIAL THANKS TO<br>
<br>
<a href="mailto:ap@webkit.org">ap@webkit.org</a> for encouraging me to instrument where it helps.<br>
<a href="mailto:cbentzel@chromium.org">cbentzel@chromium.org</a> for putting up with my yak shaving.<br>
<a href="mailto:eroman@chromium.org">eroman@chromium.org</a> for showing me how to use windb(a)g.<br>
<a href="mailto:japhet@chromium.org">japhet@chromium.org</a> for never giving up hope.<br>
</blockquote></div><br>