Skip to content

Commit e96a260

Browse files
committed
Removes special treatment of SystemExit
Previously, SystemExit exceptions were being caught at the module level, presumably to not crash an interpreter which did not expect this exception. However, this had several problems: 1. Importing a module which calls exit() in its body would not correctly exit the importing module. 2. The additional try-catch logic required for the special treatment of SystemExit had a noticeable performance penalty. 3. Special casing SystemExit on a module level is inconsistent with Python's behaviour, which treats it like any other exception, and where it is up to the interpreter (not the module) to catch this exception and deal with it as desired. 4. Additionally, SystemExit was incorrectly set to inherit from Exception (instead of BaseException), which meant that except blocks catching Exception would also catch SystemExit This patch fixes the above issues by: 1. Removing the special check for SystemExit 2. Changing SystemExit to inherit from BaseException (and, while I was at it, change other exceptions to inherit from SystemError) 3. Updating the tests to special case SystemExit as required 4. Adding new-style unit tests for testing the above issues
1 parent 807661a commit e96a260

File tree

7 files changed

+187
-96
lines changed

7 files changed

+187
-96
lines changed

src/compile.js

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -927,7 +927,7 @@ Compiler.prototype.outputSuspensionHelpers = function (unit) {
927927
}
928928
}
929929

930-
output += "try { $ret=susp.child.resume(); } catch(err) { if (!(err instanceof Sk.builtin.Exception)) { err = new Sk.builtin.ExternalError(err); } err.traceback.push({lineno: currLineNo, colno: currColNo, filename: '"+this.filename+"'}); if($exc.length>0) { $err=err; $blk=$exc.pop(); } else { throw err; } }" +
930+
output += "try { $ret=susp.child.resume(); } catch(err) { if (!(err instanceof Sk.builtin.BaseException)) { err = new Sk.builtin.ExternalError(err); } err.traceback.push({lineno: currLineNo, colno: currColNo, filename: '"+this.filename+"'}); if($exc.length>0) { $err=err; $blk=$exc.pop(); } else { throw err; } }" +
931931
"};";
932932

933933
output += "var $saveSuspension = function(child, filename, lineno, colno) {" +
@@ -1586,7 +1586,7 @@ Compiler.prototype.buildcodeobj = function (n, coname, decorator_list, args, cal
15861586
this.u.switchCode = "while(true){try{"
15871587
this.u.switchCode += this.outputInterruptTest();
15881588
this.u.switchCode += "switch($blk){";
1589-
this.u.suffixCode = "} }catch(err){ if (!(err instanceof Sk.builtin.Exception)) { err = new Sk.builtin.ExternalError(err); } err.traceback.push({lineno: currLineNo, colno: currColNo, filename: '"+this.filename+"'}); if ($exc.length>0) { $err = err; $blk=$exc.pop(); continue; } else { throw err; }} }});";
1589+
this.u.suffixCode = "} }catch(err){ if (!(err instanceof Sk.builtin.BaseException)) { err = new Sk.builtin.ExternalError(err); } err.traceback.push({lineno: currLineNo, colno: currColNo, filename: '"+this.filename+"'}); if ($exc.length>0) { $err = err; $blk=$exc.pop(); continue; } else { throw err; }} }});";
15901590

15911591
//
15921592
// jump back to the handler so it can do the main actual work of the
@@ -2225,8 +2225,6 @@ Compiler.prototype.cmod = function (mod) {
22252225
this.u.varDeclsCode += "if (typeof Sk.lastYield === 'undefined') {Sk.lastYield = new Date()}";
22262226
}
22272227

2228-
this.u.varDeclsCode += "try {";
2229-
22302228
this.u.varDeclsCode += "if ("+modf+".wakingSuspension!==undefined) { $wakeFromSuspension(); }" +
22312229
"if (Sk.retainGlobals) {" +
22322230
" if (Sk.globals) { $gbl = Sk.globals; Sk.globals = $gbl; $loc = $gbl; }" +
@@ -2245,8 +2243,8 @@ Compiler.prototype.cmod = function (mod) {
22452243
this.u.switchCode = "while(true){try{";
22462244
this.u.switchCode += this.outputInterruptTest();
22472245
this.u.switchCode += "switch($blk){";
2248-
this.u.suffixCode = "} }catch(err){ if (!(err instanceof Sk.builtin.Exception)) { err = new Sk.builtin.ExternalError(err); } err.traceback.push({lineno: currLineNo, colno: currColNo, filename: '"+this.filename+"'}); if ($exc.length>0) { $err = err; $blk=$exc.pop(); continue; } else { throw err; }} }" +
2249-
" }catch(err){ if (err instanceof Sk.builtin.SystemExit && !Sk.throwSystemExit) { Sk.misceval.print_(err.toString() + '\\n'); return $loc; } else { throw err; } } });";
2246+
this.u.suffixCode = "}"
2247+
this.u.suffixCode += "}catch(err){ if (!(err instanceof Sk.builtin.BaseException)) { err = new Sk.builtin.ExternalError(err); } err.traceback.push({lineno: currLineNo, colno: currColNo, filename: '"+this.filename+"'}); if ($exc.length>0) { $err = err; $blk=$exc.pop(); continue; } else { throw err; }} } });";
22502248

22512249
// Note - this change may need to be adjusted for all the other instances of
22522250
// switchCode and suffixCode in this file. Not knowing how to test those

src/env.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,8 @@ Sk.configure = function (options) {
4646
Sk.inputfun = options["inputfun"] || Sk.inputfun;
4747
goog.asserts.assert(typeof Sk.inputfun === "function");
4848

49-
Sk.throwSystemExit = options["systemexit"] || false;
50-
goog.asserts.assert(typeof Sk.throwSystemExit === "boolean");
51-
5249
Sk.retainGlobals = options["retainglobals"] || false;
53-
goog.asserts.assert(typeof Sk.throwSystemExit === "boolean");
50+
goog.asserts.assert(typeof Sk.retainGlobals === "boolean");
5451

5552
Sk.debugging = options["debugging"] || false;
5653
goog.asserts.assert(typeof Sk.debugging === "boolean");

0 commit comments

Comments
 (0)