User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0 Command aliases can refer to themselves, intially causing a slowdown, as infinite loops tend to do. The "a script is taking a long time - discontinue it?" dialogue is displayed after a while which allows the command to be terminated, although responsiveness of computer is already in a state where a freeze of Firefox has been known to happen. Reproducible: Always Steps to Reproduce: 1. Write a recursive command alias (e.g. back=back;away;nick mjec) 2. Run it (i.e. type /back) Actual Results: Computer response slows down, Chatzilla stops responding. Eventually script can be terminated by "this script is taking a long time" dialogue. In some cases browser stops responding altogether, sometimes (though generally only with excessive other CPU/RAM use) system crashes. Hey, it's Windows. Expected Results: Not allowed the command to run (e.g. with message "Recursive command aliases create infinite loops and they are a Bad Thing(tm); /back not run"). Chatzilla /does/ say that it doesn't like the command but continues running it anyway :/.
There are a couple of ways I can see of solving this: - check the call stack when running a command to see if we're recursing - simply count the recurse depth in a command manager variable - mark the commands themselves as 'running' temporarily The last one seems good, but we need to allow for at least some controlled recursion: the /away or /back commands when run on the client view will call themselves for each network.
(In reply to comment #1) > - mark the commands themselves as 'running' temporarily > > The last one seems good, but we need to allow for at least some controlled > recursion: the /away or /back commands when run on the client view will call > themselves for each network. I like this one, but I think it would be safe to make it only apply to aliases? That way the controlled recursion of those commands is no problem. It would already catch the alias recursion in the act, and if the built-in commands recurse badly, then something much worse is going on (and bugs should be filed & fixed).
I think option two is the way to go. Have dispatch() increment something on the command manager when it starts and decrement on the way out. If the count ever gets higher than, say, commandManager.maxDispatchDepth, then throw an exception.
Created attachment 227895 [details] [diff] [review] Enforce max dispatch depth and unwind when hit This took a little more magic that I had expected, due to the fact that displaying a message requires the command create-tab-for-view to run, so I had to make it unwind the entire stack before displaying the message at the end. It works, though, and the break when unwinding means that any aliases it is in the middle of will NOT run the rest of their code - this means it effectively kills the execution, which I think is probably the best solution.
Attachment #227895 - Flags: review?(samuel)
Checked in --> FIXED.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.