7 ms·
This bug, the HN thread and how hard it is to fix (https://bugs.python.org/issue39318 https://bugs.python.org/issue39318) is a pretty good argument against exce
by kjksf 7y ago
This bug, the HN thread and how hard it is to fix (https://bugs.python.org/issue39318 https://bugs.python.org/issue39318) is a pretty good argument against exceptions.
A code that looks simple and correct has been hiding a very subtle bug because no one understood the implications of an arbitrary exception being thrown at any point under the caller.
And of all the people who looked at the bug not a single one looked at this seemingly simple code and said "oh yeah, that's because this code is obviously wrong". It took someone skilled enough to script gdb to figure out the buggy interactions.
So the next time someone makes fun of `if err != nil`, send them the buggy implementation of tempfile.NamedTemporaryFile and ask them to spot the bug.
Also, KeyboardInterrupt should not be an exception. It's a signal, not an error or exceptional situation that code wants to report to the caller. But once you have a hammer, everything looks like like a nail. In addition to making the code impossible to reason about, no-one can agree what exactly is an exception so they randomly commingle expected errors with exceptional circumstances and, in case of Python, out-of-band signals.
("randomly" as in: why is "open file" throwing an exception when file doesn't exist but "find substring" returns -1 when substring doesn't exist?)
- excpointer 7y agoPopcorn time...
- deanCommie 7y agoAn extremely obscure and rare scenario that exposes a flaw with a convenient tool does not negate the value of the tool in all other cases. Are you also going to throw out all use of automatic garbage collection in languages because of a bug caused by a GC pause. Should we stop bothering with encryption because people discover weaknesses and attack vectors? Come on, man. > Also, KeyboardInterrupt should not be an exception. It's a signal, not an error or exceptional situation that code wants to report to the caller. OP doesn't mention anything about KeyboardInterrupt, but I'll bite. Some languages like C# make "Events" a 1st party citizen, making it really easy to write non-sequential code and handling such events. Most don't, so when people want to GUARANTEE a caller handles a particular signal, they use the Exception tool because it posesses the necessary qualities. "Using Exceptions as flow control" is a well-known anti-pattern but even anti-patterns have their use in exceptional circumstances. > ("randomly" as in: why is "open file" throwing an exception when file doesn't exist but "find substring" returns -1 when substring doesn't exist?) I'll concede this one point. In-memory utility methods shouldn't throw exceptions when a useful error code would be just as good - it keeps code cleaner without trycatch blocks. NumberFormatException is my bane in Java.
- CaptainMarvel 7y agoExceptions as flow-control is normal in Python. It’s used all over the place, e.g. generators with StopIteration.
- rumanator 7y ago> Exceptions as flow-control is normal in Python. Perhaps it's high time that problem was solved?
- maweki 7y agoEspecially on StopIteration this is a good way to keep the interface small when lacking static typing. A generator in Java has the has_next() and next() method and this can be statically checked and you're basically only allowed to call next is hasnext is true. In python the generator only has to provide the next-method and raises an exception when it's exhausted. I don't find this particularly unclean.
- larkost 7y agoThis isn't a problem, it is a design decision that you evidently don't like. There is a difference.
- minitech 7y agoAs a fan of Python, I agree that it’s a problem, and probably one of Python’s worst design decisions. It doesn’t achieve anything over sentinels, and results in lots of potential for hidden bugs (especially in the case of __getattr__, and before PEP 479). Comparison overloading even does use NotImplemented instead of an exception.
- rumanator 7y ago> This isn't a problem It actually is. Abusing exceptions as the normal control flow is a crude anti-pattern. Obviously the normal flow of control is not exceptional behavior. Therefore, abusing exceptions and exception-handling mechanisms to implement the happy path is simply wrong at many levels, from conceptual to practical, and in the end actually cause problems and hard to find buga such as the problems described in this bug report.
- simion314 7y agoI do not see how exceptions caused the bug, seems the cause is a bug in a GC language d that is wrapping a low level code/resource and not releasing it properly.
- masklinn 7y agoIn a langage without exceptions the file wrapper being trivial it would not have returned any sort of error condition and thus would not have been checked for it. It is an issue of exceptions that try clauses can easily be overly broad in the amount of code they cover.
- simion314 7y agoI still do not understand, I did not see where the exception caused the issue in this case, you can have bugs with error codes that are not checked and that blow up much later so I am confused by your vague response (maybe you prefer checked excpetion so you don't ignore problems, I prefer those because they force correctness over developer comfort).
- masklinn 7y ago> I did not see where the exception caused the issue in this case try: file = _io.open(fd, mode, buffering=buffering, newline=newline, encoding=encoding, errors=errors) return _TemporaryFileWrapper(file, name, delete) except BaseException: _os.unlink(name) _os.close(fd) raise The except clause was built under the assumption that only opening the fd would fail, and thus the cleanup it performs is in those terms, especially the part about closing the file descriptor and ignoring the file object (since that's assumed not to possibly exist). However due to the mock it's the wrapper creation which fails. This means the cleanup runs, closes the fd, but leaves the file object (created by io.open) around and assuming it owns an fd, the file object later on gets collected by the GC and closes the fd a second time, breaking anyone unlucky enough to have gotten its reuse. This issue would not happen if the wrapper were created outside the `try` statement (though the file would be leaking to be eventually closed by the GC which is not great either)
- joshuamorton 7y ago> ("randomly" as in: why is "open file" throwing an exception when file doesn't exist but "find substring" returns -1 when substring doesn't exist?) Because there can be lots of ways for `open` to throw an exception (encoding issues, not found, permissions, etc.). The underlying open syscall has 39 error codes. Python adds a few more failure modes atop that. I'd much rather deal with named error conditions than deal with return values -1 through -50. Find substring can only fail in one way.
- incompatible 7y agoEven C can map the returns values -1 through -50 to symbolic constants, typically defined in preprocessor headers.
- joshuamorton 7y agoAbsolutely, but dealing with categories of errors is much harder with unstructured error codes.
- ken 7y ago> a pretty good argument against exceptions Every time there’s a subtle bug with 7 contributing factors, people pick the one which is caused by a design decision they happen to hate, and place all of the blame squarely on that one.
- winstonewert 7y agoMy question is: what is your alternative to exceptions, and how would they would have made the error easier to spot? Its not obvious that the error would have been easier to spot if it had been written using Go's `!= nil` pattern or Rust's `?` pattern. My experience is that while its true that its easy to have bugs in implicit exception handling logic, its just as easy, maybe easier, to have bugs in the explicit boilerplate its replacing.
- imron 7y ago> Its not obvious that the error would have been easier to spot if it had been written using Go's `!= nil` pattern or Rust's `?` pattern. I like Rust's `?` operator (and Swift's `guard` statement also), but it's worth noting that the bug wouldn't have existed in Rust in the first place because the file object wouldn't have outlived the scope it was defined in.
- masklinn 7y agoAlso and really because it’s quite hard to go and replace implementation details with code which behaves completely differently and implements an unrelated contract entirely.
- Dylan16807 7y ago> how would they would have made the error easier to spot? There was a try block with two function calls, and it seems like the author assumed only the first function call could throw. With return-based errors, the error handling wouldn't implicitly be shared between the two functions. The author would write the handling for the first function, and then they would notice the second function could return an error state too. Then they might write a correct handler, or at least we should expect them to say "this can't happen, so panic". This bug would have been trivial to fix if it panicked instead of corrupting fds.
- winstonewert 7y agoWhat makes you think that "the author assumed only the first function call could throw?" Programmers in exception languages never think (or at least should never think) "X cannot throw" because it is never true. Any function has the potential of throwing an exception. This is even more true in Python than in other exception throwing languages. No, the problem here is that the author has realized that they need special cleanup logic in the case that a file fails to be opened, but they applied that logic too broadly.
- thaumasiotes 7y ago> why is "open file" throwing an exception when file doesn't exist but "find substring" returns -1 when substring doesn't exist? Well, first, there's more to it than that. "Open file" will throw an exception when you open the file to read and it doesn't exist. If you open the file to write and it doesn't exist, you won't get an exception because that's expected behavior. Instead, the file will be automatically created behind the scenes. The complementary scenario, where you're opening a file to write and the file already exists, is more likely to be a bug. Exceptions have semantic qualities and depend on what you're trying to do. Trying to read a file that doesn't exist is obviously an error state because it is inherently impossible to do. The same goes for detecting the location of a substring within a string that doesn't contain it. But examining a string to see whether it contains a particular substring has no such problem. You can always test a string for the inclusion of some other string. So the judgment of whether "find substring" should throw an exception or not depends on whether you think the question being asked when someone invokes "find substring" is "is substring in there?" (answer of "no" is not exceptional) or "I know substring is in there; where is it?" (answer of "it isn't in there, you doofus" is exceptional). Both questions are common; this isn't really a call the language should be making. The same problem is more frequently discussed in the context of hash tables. If I look up a key in a hash table and I get a null value, is that because there is no value stored for that key, or is it because null is the value stored for that key? People would like accessing the table to be convenient while distinguishing these two cases, and python doesn't allow that. You can choose from some different behaviors: table[key] # Absence of the key is exceptional. table.get(key) # Absence of the key will yield None... # but this is indistinguishable from # a key with the stored value None. key in table # This directly tests whether key is # present, but won't tell you the stored value. The true solution is for lookups to return multiple values, one to tell you whether the key was present and the other to tell you what its value was, if anything. But this is rare[1] because people don't want to handle multiple return values from every hash lookup. But you can store any value, even null, in a hash table -- that's their thing. Finding substrings doesn't have this problem; a legal result must be a valid index into the string. We can solve the how-do-we-want-the-hash-table-to-behave problem perfectly by returning an invalid index when the substring isn't present, and... that's what we do. [1] Common Lisp addresses this by allowing functions to return multiple values (distinct from a single value with multiple subvalues) -- it's possible to treat the return value of a multiple-valued function as a single value, in which case you get only the first value. This is great for the hash table example, but in general it makes this easier while making it much more annoying to handle the multiple values when you do want them all.
- mixmastamyk 7y agoA non-sequitur me thinks, as the code doesn't look simple and correct. File needs to be closed on error if it exists, fd if not. i.e. The broken version looks broken. While there is lots of meta-complexity around this, the bug itself is relatively simple. Make sure files are closed properly on error. Also find -1 is a wrapper from C, probably kept for compatibility reasons.