Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#1260 closed defect (fixed)

[PATCH] prevent dojo.declare stack corruption due to unhandled exceptions

Reported by: newmanb@… 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)

declare.diff (737 bytes) - added by newmanb@… 13 years ago.
same as the diff pasted above, but perhaps easier to apply

Download all attachments as: .zip

Change History (5)

Changed 13 years ago by newmanb@…

Attachment: declare.diff added

same as the diff pasted above, but perhaps easier to apply

comment:1 Changed 13 years ago by newmanb@…

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 bill

Owner: changed from anonymous to sjmiles

comment:3 Changed 13 years ago by sjmiles

Milestone: 0.4
Resolution: fixed
Status: newclosed

Agree with analysis, added try/catch/finally block to dojo.declare._inherited.

comment:4 Changed 13 years ago by (none)

Milestone: 0.4

Milestone 0.4 deleted

Note: See TracTickets for help on using tickets.