Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork939
Commit3f21391
committed
Remove unused TASKKILL fallback in AutoInterrupt
When Git.AutoInterrupt was first implemented inead94f2, it usedos.kill (sending SIGINT to the process to terminate it). This wasin 2009, and not all supported versions of Python provided os.killon Windows, since it was added for Windows in 2.7 and 3.2 (2.6 and3.1 reached EoL in 2013 and 2012, respectively). CatchingAttributeError and running TASKKILL provided a fallback for Pythonversions in which os.kill was absent on Windows.Since then, all supported versions have os.kill on Windows, andalso the code of GitPython, in the try-block, has changed, and nolonger uses os.kill. It now contains four attribute accessexpressions:- proc.terminate. All currently suppported versions of Python (including 3.7, which GitPython still supports, and some before that) have Popen.terminate.- proc.wait. Same situation as proc.terminate. Popen.wait exists.- self._status_code_if_terminate. This is a class attribute of AutoInterrupt, accessible through its instances.- self.status. This is assigned to. AutoInterrupt is slotted, but "status" appears in __slots__, so this isn't an AttributeError either.The "except AttributeError" clause is thus no longer used and canbe removed, which is done here. Removing it shortens and simplifiesthe code, better expresses the logic for how the wrapped process isactually being terminated, and eliminates the need to engage withany subtleties of TASKKILL (and of how it is called) when readingthe code to verify its correctness.In addition, because they are now expected always to be available,if somehow an AttributeError managed to be raised in the terminateor wait calls, this would be very strange and we probably shouldn'tsilently catch that.(Because this AutoInterrupt._terminate method is sometimes calleddue to __del__ methods running as the interpreter is shutting down,it is possible for some attributes to unexpectedly be set to None,which could cause AttributeError indirectly if another attribute islooked up on the None object; and perhaps on some non-CPythonimplementations some attributes might even be deleted duringshutdown. The _terminate method handles this where relevant. Butthe TASKKILL fallback removed here seems unrelated to that, whichaffects module attributes and global variables. The names used inthe try-block are proc, status, and self, which are localvariables; the except clause itself accessed os.name, which used aglobal variable and a module attribute, indicating that the intentwas not to handle this interpreter shutdown scenario; and thiswhole issue, while not gone completely, is much less significantsince Python 3.4, due tohttps://docs.python.org/3/whatsnew/3.4.html#whatsnew-pep-442.)1 parenta30b3b7 commit3f21391
1 file changed
+1
-11
lines changedLines changed: 1 addition & 11 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
14 | | - | |
| 14 | + | |
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| |||
544 | 544 | | |
545 | 545 | | |
546 | 546 | | |
547 | | - | |
548 | | - | |
549 | | - | |
550 | | - | |
551 | | - | |
552 | | - | |
553 | | - | |
554 | | - | |
555 | | - | |
556 | | - | |
557 | 547 | | |
558 | 548 | | |
559 | 549 | | |
| |||
0 commit comments
Comments
(0)