Thanks for sharing the code John. This was indeed a bug in the handling of termination exceptions. I have fixed the problem in V8 bleeding_edge and will push it to trunk and into chromium ASAP.
On Tue, Sep 1, 2009 at 7:34 AM, Mads Sig Ager<a...@chromium.org> wrote: > v8::V8::TerminateExecution is the right thing to call. You have to > propagate the termination exception all the way out past the last > exception handler on the stack for it to be cleared again. If you > call into V8 when the termination exception is still active it will be > rethrown. If that happens in a context creation, it will probably > lead to a crash. > > Could you share the code with me (and instructions on reproducing the > crash) so I can have a quick look at it? > > Thanks, -- Mads > > On Tue, Sep 1, 2009 at 3:49 AM, John Abd-El-Malek<j...@chromium.org> wrote: >> >> btw I've updated my patch with the feedback below. I'm now leaning >> towards stopping execution of the script instead of skewing time. I >> tried using v8::V8::TerminateExecution in a C++ callback, but then on >> the next navigation v8 crashes. Is this the right method to call? If >> not, what should I call? >> >> Thanks, >> John >> >> On Aug 31, 12:48 pm, John Abd-El-Malek <j...@chromium.org> wrote: >>> (just saw all these emails, as I'm not on v8-dev and Groups kicked me >>> off the cc list). >>> >>> re Dean's comment: the newest version of the code doesn't have any >>> exposed functions. Yes it could be worked around, i.e. by computing >>> primes. But the point of this change is not to eliminate any possible >>> way of JS burning up CPU. It's simply to get rid of the simulated >>> sleep() calls in unload handlers, which cause our UI to appear janky >>> when closing a tab, or the browser to appear slow when navigating >>> between pages. The plan is to implement <ping> in the meantime so >>> that web developers have an alternative that doesn't slow down the >>> user. >>> >>> re Mike: I think expecting users to know what unload handlers are, or >>> which ones should be allowed, is not realistic. >>> >>> re Mads: Good point re the Date constructor, I'll change the current >>> patch to hide it. >>> >>> I don't have strong feelings of whether we should skew the time or >>> just stop the script if we detect a "sleep" pattern. I do think we >>> need to do something. >>> >>> On Aug 31, 10:50 am, Darin Fisher <da...@chromium.org> wrote: >>> >>> >>> >>> > On Mon, Aug 31, 2009 at 9:05 AM, Mads Sig Ager <a...@chromium.org> wrote: >>> >>> > > The invalid assert has now been fixed. I still don't think we should >>> > > do this for the reasons that are in this thread. >>> >>> > > Another issue is that developers can replace the Date constructor >>> > > anyway they like. If the developer does that, then the replacement >>> > > for Date will not have the new method and attempting to call it will >>> > > throw an exception that we need to handle somehow. If we handle that >>> > > gracefully, the end result will be that there is no change to the Date >>> > > constructor. So a very simple workaround for a developer who wants a >>> > > sleep in the unload handler is to replace the Date constructor with >>> > > some other function that uses the real Date constructor to get the >>> > > time. >>> >>> > Why can't we hide the real Date constructor? Can't we implement this as a >>> > lower level? >>> >>> > I'm somewhat sympathetic to the viewpoint that the suggested change will >>> > only lead to an arms race. Afterall, that's what happened with popup >>> > blocking. Now, instead of popups, we have floating divs. Floating divs >>> > are >>> > probably a bit better (since they are scoped to a tab), but there is no >>> > way >>> > to block them. The browser loses any insight into what the page is trying >>> > to do. >>> >>> > Learning from the lesson of popup blocking, what we should be sure to do >>> > here is to provide a better tool to these "bad guys" so that they can do >>> > what they want in a less detrimental-to-the-user way. In this case, I >>> > think >>> > we need to offer support for <a ping> or something like it and encourage >>> > its >>> > use. Then, once that exists, it is easy to kill scripts that spin a tight >>> > loop querying time of day during an unload handler. Anyone who finds >>> > their >>> > scripts broken by that will most likely switch to <a ping> instead of >>> > implementing some other CPU consuming algorithm (computing prime >>> > numbers?). >>> >>> > -Darin >>> >>> > > Cheers, -- Mads >>> >>> > > On Sat, Aug 29, 2009 at 11:34 PM, Mike Belshe<mbel...@google.com> wrote: >>> > > > I agree with Dean on this one. Maybe it's a fool's errand to even try >>> > > > anything. >>> > > > My first choice is to nuke the unload handlers. However, there are >>> > > > legitimate sites saying "you're about to lose your data", so that >>> > > > doesn't >>> > > > work. >>> > > > My second choice would be to have users give approval for unload >>> > > handlers, >>> > > > or auto detect which are junk, but I can't think of any reasonable >>> > > > algorithms to do this? Could we have a cloud-based vote system? >>> > > > Maybe >>> > > that >>> > > > is all junk too. >>> > > > Lastly, my thought is to change the UI. What if we allow the page >>> > > > transition, immediately, and on the new window you get one of those >>> > > yellow >>> > > > infobars saying "the last page did not close" with buttons to "<view >>> > > > it>" >>> > > or >>> > > > "<close it>". Optionally, if the page was good enough to use one of >>> > > > our >>> > > > alert dialogs, we could even display the page's message in there. >>> > > Hopefully >>> > > > this would get our page transitions faster. >>> > > > I'd be willing to bet there are more onunload handlers I don't want to >>> > > run >>> > > > than ones I do want to run. To Dean's point, even trying to do what >>> > > > I'm >>> > > > suggesting may just force these folks into even worse tricks to try to >>> > > track >>> > > > the user. >>> > > > chrome needs adblock, i guess. >>> > > > Mike >>> >>> > > > On Fri, Aug 28, 2009 at 9:41 AM, Dean McNamee <de...@chromium.org> >>> > > wrote: >>> >>> > > >> John, >>> >>> > > >> If we pushed out the getTime change, it will be easily to realize >>> > > >> what >>> > > >> we've done and quickly adapt to doing something different. Basically >>> > > >> I think it's an ugly hack that will have a small and likely temporary >>> > > >> effect on the real problem. >>> >>> > > >> On Fri, Aug 28, 2009 at 9:00 AM, John Abd-El-Malek <j...@google.com> >>> > > wrote: >>> > > >> > Thanks Mads for the feedback, comments below. >>> >>> > > >> > On Fri, Aug 28, 2009 at 2:50 AM, Mads Sig Ager <a...@chromium.org> >>> > > >> > wrote: >>> >>> > > >> >> John, >>> >>> > > >> >> I'll look into the assertion failure. I'm not sure why you are >>> > > >> >> hitting that one - that assert is exactly setup that way to allow >>> > > eval >>> > > >> >> in extensions but not in our own native function code. >>> >>> > > >> >> That said, I think this is a pretty nasty hack and I think we >>> > > >> >> should >>> > > >> >> avoid putting it in if we can. There are so many ways users can >>> > > >> >> make >>> >>> > > >> >> stuff in onunload handlers take a long time. >>> >>> > > >> > I agree it's a hack and I would rather not put it in either. >>> > > >> > However >>> > > we >>> > > >> > have a problem in that we see web pages sleeping for 2 seconds. >>> > > >> > The >>> > > end >>> > > >> > effect is that it looks like Chrome is slow. The user can't tell >>> > > >> > that >>> > > >> > it's >>> > > >> > the page or the the browser, but they have given us the signal that >>> > > they >>> > > >> > want to leave the page. Making them wait a few seconds gives a >>> > > >> > poor >>> > > >> > experience, i.e. see all the comments athttp://crbug.com/7823. >>> >>> > > >> >> Lying about the time to >>> > > >> >> avoid one case seems like a bad idea. I'm not sure I agree that >>> > > >> >> it >>> > > is >>> > > >> >> the browser's problem that a user decides to perform long-running >>> > > >> >> operations in onunload. >>> >>> > > >> > Just to make things clear, which user are you talking about, the >>> > > person >>> > > >> > using the browser or the person who wrote the slow page? The >>> > > >> > person >>> > > >> > surfing >>> > > >> > has no idea about how the page is written, and just made a >>> > > >> > decision to >>> > > >> > leave >>> > > >> > the page. If it was up to them, they wouldn't care to stay on a >>> > > >> > page >>> > > >> > for a >>> > > >> > few seconds to let it do whatever tracking it wants. >>> >>> > > >> > For the person writing the page, there are better ways of doing >>> > > >> > this. >>> > > >> > i.e. >>> > > >> > they can have a bunch of handlers that wait progressively more and >>> > > more >>> > > >> > time, so that people with fast connections still don't have to wait >>> > > this >>> > > >> > long. Or as browser developers, we can provide them with better >>> > > methods >>> > > >> > of >>> > > >> > doing this, so they don't slow down navigations at all. We're >>> > > >> > going >>> > > to >>> > > >> > do >>> > > >> > that with <a ping>, which is in HTML5. Once that's implemented, we >>> > > will >>> > > >> > do >>> > > >> > developer outreach to get them to use it instead of the sleep >>> > > >> > hacks. >>> >>> > > >> >> If we believe that the browser should >>> > > >> >> disallow that, maybe we should explicitly set a timout after which >>> > > >> >> javascript execution in onunload will be forcefully terminated? >>> > > >> >> That >>> > > >> >> will probably have other issues though. >>> >>> > > >> > We discussed a bunch of these options (I should have sent a link to >>> > > the >>> > > >> > bug >>> > > >> > in the message earlier, it goes through some of the problems of >>> > > >> > doing >>> > > >> > something like this). The problem is that this opens up a can of >>> > > worms >>> > > >> > in >>> > > >> > terms of site compatibility. Some sites (i.e. docs) might do a >>> > > >> > sync >>> > > XHR >>> > > >> > to >>> > > >> > save the draft of the document, while others like Gmail might pop >>> > > >> > up a >>> > > >> > dialog box if there's unsaved emails. Any timeout would have false >>> > > >> > positives on legitimate code depending on the machine hardware, and >>> > > >> > would >>> > > >> > break some sites. This approach works, IMO, because I can't find >>> > > >> > any >>> > > >> > legitimate site that needs to call getTime 1000 (or even 10000 if >>> > > >> > we >>> > > >> > choose >>> > > >> > that instead) times in an unload handler. It's a surgical way to >>> > > >> > get >>> > > >> > rid of >>> > > >> > sleeps. >>> > > >> > I look at this as simulating having a slow cpu and latency. i.e. >>> > > >> > when >>> > > >> > the >>> > > >> > script calls getTime, the cpu runs a different process and returns >>> > > after >>> > > >> > 1ms. After the sleeps loop is done, the request is still pending >>> > > >> > because of >>> > > >> > a slow network connection. The site already has to deal with >>> > > >> > machines >>> > > >> > like >>> > > >> > this, so we're not breaking them. >>> >>> > > >> > I'm open to any other solutions that people can think of which have >>> > > less >>> > > >> > side-effects. >>> > > >> > Thanks, >>> > > >> > John >>> >>> > > >> >> Cheers, -- Mads >>> >>> > > >> >> On Fri, Aug 28, 2009 at 5:06 AM, John >>> > > >> >> Abd-El-Malek<j...@google.com> >>> > > >> >> wrote: >>> > > >> >> > Hi, >>> > > >> >> > In investigating a bug where tabs take a long time to close, I >>> > > >> >> > tracked >>> > > >> >> > the >>> > > >> >> > issue to sites simulating sleep in their unload handlers. They >>> > > >> >> > do >>> > > >> >> > this >>> > > >> >> > by >>> > > >> >> > looping until Date.getTime changes by enough ms. I'm now >>> > > prototyping >>> > > >> >> > disabling these sleeps by skewing the time after a large number >>> > > >> >> > of >>> > > >> >> > getTime >>> > > >> >> > calls in an unload handler. Here's the changelist where I >>> > > >> >> > implemented >>> > > >> >> > it >>> > > >> >> > using a V8 >>> >>> ... >>> >>> read more » >> >> >> > --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
