#1260 closed defect (fixed)
[PATCH] prevent dojo.declare stack corruption due to unhandled exceptions
Reported by: | Owned by: | sjmiles | |
---|---|---|---|
Priority: | blocker | Milestone: | |
Component: | Core | Version: | 0.3 |
Keywords: | declare stack corruption exceptions | Cc: | |
Blocked By: | Blocking: |
Description
The function dojo.lang.declare.base._inherited
wraps each method of a declared class in another function which keeps track of the prototype of the receiving object while calling the original method. The prototype needs to be reset to its original value after the function call, but if an unhandled exception is thrown inside the original method, the reset step will be skipped. This (usually silent) failure leads to really nasty inheritance bugs.
I propose a simple fix: wrap the method call in a try-catch
block, catch and re-throw all exceptions, but include a finally
clause to ensure that the prototype will be reset no matter what happens. Just another step towards making the _inherited
function invisible to the client.
If dojo.declare
is going to see as much use as I anticipate, this pitfall really has to be fixed. I gave it a high priority because the fix is so simple, and yet (IMHO) pretty important.
Here's the patch:
Index: src/lang/declare.js =================================================================== --- src/lang/declare.js (revision 5211) +++ src/lang/declare.js (working copy) @@ -122,10 +122,11 @@ _getPropContext: function() { return (this.___proto||this); }, // caches ptype context and calls method on it _inherited: function(ptype, method, args){ - var stack = this.___proto; + var result, stack = this.___proto; this.___proto = ptype; - var result = ptype[method].apply(this,(args||[])); - this.___proto = stack; + try { result = ptype[method].apply(this,(args||[])); } + catch (e) { throw e; } + finally { this.___proto = stack; } return result; }, // invokes ctor.prototype.method, with args, in our context
Attachments (1)
Change History (5)
Changed 13 years ago by
Attachment: | declare.diff added |
---|
comment:1 Changed 13 years ago by
Summary: | prevent dojo.declare stack corruption due to unhandled exceptions → [PATCH] prevent dojo.declare stack corruption due to unhandled exceptions |
---|
comment:2 Changed 13 years ago by
Owner: | changed from anonymous to sjmiles |
---|
comment:3 Changed 13 years ago by
Milestone: | → 0.4 |
---|---|
Resolution: | → fixed |
Status: | new → closed |
Agree with analysis, added try/catch/finally block to dojo.declare._inherited.
same as the diff pasted above, but perhaps easier to apply