Closed Bug 2295 Opened 26 years ago Closed 26 years ago

Viewer crash in nsTimerMac

Categories

(Core :: Layout, defect, P1)

PowerPC
All
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: sfraser_bugs, Assigned: mozeditor)

References

()

Details

The following will reliably crash in nsTimerMac: Run the viewer, type 'http://w3' into the URL field, and hit return. Once W3 loads, click on the link at the top to go to the no layers, no stylesheets version. It then crashes in TimerImpl::Fire() (looks like the mCallbackObject has been freed).
Status: NEW → ASSIGNED
I think mcmullen's recent changes to nsTimerMac fixed this. I'll leave it for Joe to check after his changes.
I think mcmullen's recent changes to nsTimerMac fixed this. I'll leave it for Joe to check after his changes.
Yea, this would be the same bug I fixed. #2164. Firing timers of any kind would previously shoot you in the phoot.
Status: ASSIGNED → RESOLVED
Closed: 26 years ago
Resolution: --- → FIXED
We were leaking every GraphicState that was ever made on behalf of an nsRenderingContextMac. And then we were runing out of mem in PowerPlant code, which threw an exception which caused immediate program termination (since we aren't catching excpetions - since we aren't supposed to be using them). To solve part be of this problem I removed all Powerplant usage (see bug 2246). Solving the first part was a pair of simple changes in the destructor of nsRenderingContextMac.
oops, wrote the previous comments to the wrong bug. this one is still fixed though, due to McMulen's earlier efforts.
Status: RESOLVED → REOPENED
OS: All
Priority: P2 → P1
I'm seeing a crash in the timer code perhaps 50% of times that I run. Now, with the memory allocators setting freed memory to 0xEFEFEFEF, this is much more apparent. I usually see this crash when Sample 0 is loading on starting the viewer. This is almost a development-stopper for me.
Simon and I spent some quality time together tonight and figured out what was happening. He has a bad memory card in his machine. No, just kidding. I think we have figured out what is happening and it is a design flaw in my implementation of the mac timer code and the mac repeater code. The flaw is that the loop that fires timers (or repeaters) is not safe from changes to the list of timers (or repeaters) from within the firing (or repeat action). I'm going to fix this in two seperate ways. For timers, I'm simply going to copy the list before firing timers, and fire off of the copied list. For repeaters, I'm going to make the repeater class an intrusive linked list so that the state of the list iterators will always be valid. Thanks to Simon for all his help on this.
I'm not sure it is a good solution: you will be much more vulnerable to changes in the list if you make a copy of it, for instance if a timer destroys another one. Why didn't we have the problem with LPeriodicals? They too keep a linked list.
Status: REOPENED → RESOLVED
Closed: 26 years ago26 years ago
Pierre, what the timer code is actually doing is making an "almost" copy of the list. It is just scanning the list and removing all the timers from it that are due to fire, and putting them in a new (private) list. It then fires off the timers from the private list. They timers are addref'd as they are put on the private list, and released after they are fired - so no timer will be able to destroy one that is due to fire, until after it has. I didn't use that solution for repeaters because (unlike the timer case) it does make sense for one repeater to be able to destroy another and prevent it from firing (I have an example in mind that it too tedious to present here). Anyway, give me a call if you have questions/suggestions. i've checked this stuff in and am marking this fixed again. When Simon comes back I guess we'll find out for sure - his mac seems to drive these problems much better than mine.
Status: RESOLVED → REOPENED
I'm reopening, on the basis of my crashing much more rarely now in the same place, but still crashing, after Joe's fixes, and my fixes of Joe's fixes ;) It looked like the address of the callback in nsTimer::Fire() was bad, so the bug is probably not in the timer code, but somewhere else.
The case you are seeing is maybe a dup of #2535 "Widgets don't stop repeating when disposed->crash".
Resolution: FIXED → ---
I don't think so. I see it when viewer is loading Sample 0 for the first time.
I'm not happy with this state of affairs. The timer code needs to be rock solid. I'm going to investigate this some more.
I rewrote this stuff yet again (third time's the charm). feedback from other macheads positive so far (no one has had anymore trouble). I'm going to wait until Monday and if there is no reported problem will mark this fixed again.
Status: REOPENED → RESOLVED
Closed: 26 years ago26 years ago
Resolution: --- → FIXED
ok, so I lied. I'm not going to wait until Monday. No one (including Simon) has had any more timer probs, so I'm marking FIXED.
Status: RESOLVED → VERIFIED
Verified fixed using 2.2.99 Mac OS build. Can't reproduce bug after looping through old ---> new W3 page for a dozen iterations. (Plus, Simon also seems confident that the third time was, in fact, the charm. ;)
You need to log in before you can comment on or make changes to this bug.