Greetings! Yesterday, I committed revision r67843 to py3k. Re-enablign the windows CRT runtime checks showed me that close() was beeing called with an invalid file descriptor. Now, the problem was was in tokenizer.c, but the reason this wasn't caught earlier was, 1) Incorrect error checking for close() in _fileio.c, which I fixed, and 2) Line 384 in io.py, where all exceptions are caught for self.close(). Fixing 1 and patching 2 would bring the problem to light when running the test_imp.py part of the testsuite and, indeed, applying the fix to tokenizer.c would then remove it again. I am a bit worried about 2) thoug. I didn't modify that, but having a catch all clause just to be clean on system exit seems shaky to me. I wonder, is there a way to make such behaviour, if it is indeed necessary, just to be active when exit is in progress? Something like: try: self.close() except: try: if not sys.exiting(): raise except: pass Or better yet, do as we have done often here, just catch the particular problem that occurs during shutdown, most often name error: try: self.close() except (AttributeError, NameError): pass What do you think?
Hello, Kristján Valur Jónsson wrote:
Greetings!
Yesterday, I committed revision r67843 to py3k.
Re-enablign the windows CRT runtime checks showed me that close() was beeing called with an invalid file descriptor.
Now, the problem was was in tokenizer.c, but the reason this wasn't caught earlier was,
1) Incorrect error checking for close() in _fileio.c, which I fixed, and
2) Line 384 in io.py, where all exceptions are caught for self.close().
Fixing 1 and patching 2 would bring the problem to light when running the test_imp.py part of the testsuite and, indeed, applying the fix to tokenizer.c would then remove it again.
I am a bit worried about 2) thoug. I didn't modify that, but having a catch all clause just to be clean on system exit seems shaky to me. I wonder, is there a way to make such behaviour, if it is indeed necessary, just to be active when exit is in progress?
Something like:
try: self.close() except: try: if not sys.exiting(): raise except: pass
Or better yet, do as we have done often here, just catch the particular problem that occurs during shutdown, most often name error:
try: self.close() except (AttributeError, NameError): pass
I suggest "except Exception": SystemExit and KeyboardInterrupt inherit from BaseException, not from Exceptions And close() is likely to raise IOErrors. -- Amaury Forgeot d'Arc
try: self.close() except: try: if not sys.exiting(): raise except: pass
Or better yet, do as we have done often here, just catch the particular problem that occurs during shutdown, most often name error:
try: self.close() except (AttributeError, NameError): pass
From: Amaury Forgeot d'Arc [mailto:amauryfa@gmail.com] I suggest "except Exception": SystemExit and KeyboardInterrupt inherit from BaseException, not from Exceptions And close() is likely to raise IOErrors.
Ah, but that is not what the intent is to guard agains, according the comments. During exit, modules have been deleted and all sorts of things have gone away. It is therefore likely that code that executes during exit will encounter NameErrors (when a module is being cleaned up and its globals removed) And AttributeErrors. ImportErrors too, in fact. It would be good to see the actual repro case that caused this to be added in the first place, so that we could selectively catch those errors. Kristján
Kristján Valur Jónsson wrote:
Ah, but that is not what the intent is to guard agains, according the comments. During exit, modules have been deleted and all sorts of things have gone away. It is therefore likely that code that executes during exit will encounter NameErrors (when a module is being cleaned up and its globals removed) And AttributeErrors. ImportErrors too, in fact.
It would be good to see the actual repro case that caused this to be added in the first place, so that we could selectively catch those errors.
Generally speaking, close() and __delete__() methods that can be invoked during interpreter shutdown should avoid referencing module globals at all. Necessary globals (including members of other modules) should either be cached on the relevant class or captured in a closure. Now, it may be that the relevant close() method in io.py touches too much code for that to be practical, but it certainly isn't the case in general that encountering Name/Attribute/ImportError during shutdown is inevitable. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia ---------------------------------------------------------------
Ok, in this case I move that we remove this try/except and see where it leads us. If we see problems during teardown, we should deal with them in a more targeted manner. Kristján -----Original Message----- From: Nick Coghlan [mailto:ncoghlan@gmail.com] Sent: 19. desember 2008 13:51 To: Kristján Valur Jónsson Cc: Amaury Forgeot d'Arc; Python-Dev Subject: Re: [Python-Dev] try/except in io.py Generally speaking, close() and __delete__() methods that can be invoked during interpreter shutdown should avoid referencing module globals at all. Necessary globals (including members of other modules) should either be cached on the relevant class or captured in a closure. Now, it may be that the relevant close() method in io.py touches too much code for that to be practical, but it certainly isn't the case in general that encountering Name/Attribute/ImportError during shutdown is inevitable.
participants (3)
-
Amaury Forgeot d'Arc -
Kristján Valur Jónsson -
Nick Coghlan