Bug #915: Solved

https://nedbatchelder.com//blog/202001/bug_915_solved.html

by ingve • 7 years ago

345 105 7 years ago

nedbatchelder.com

lilSebastian• 7 years ago

Previously: https://news.ycombinator.com/item?id=22028581

egdod• 7 years ago

That was quick.

alpb2• 7 years ago

Pretty funny how the entire HN rallied together to solve some github issue on someone's personal repo. I'll try posting some of my bugs here to see if it'll work for me as well.

dang2• 7 years ago

There's a power-law dropoff when it comes to follow-up submissions, since the hivemind craves novelty. But it would be fun to try. Maybe it could become a thing: Bug HN!

The trick would be to make the bugs interesting: weird or hard, preferably both.

codezero2• 7 years ago

Bug HN might become a huge hobby for me if people follow through on that.

teej• 7 years ago

Yep, when I sent out a call for help for a bug on HN in 2012, I got ~190 votes. When I posted the followup I got 5.

It’s a little strange because for Reddit’s r/legaladvice it’s the opposite. Follow up posts can rank much more highly than the original.

SEJeff3• 7 years ago

That hard part would be preventing HN from doing people's job / homework here.

incompatible2• 7 years ago

At very least, there would have to be apparently well-written source code and an apparently competent person who can't figure out the problem.

dredmorbius2• 7 years ago

An entomology etymology ontology difficulty-novelty hierarchy?

swebs2• 7 years ago

I think the biggest dragon to slay would be the legendary GTK issue #233 which has been open for 15 years and has become sort of a meme by now. So many people try to solve it but fail since it requires intricate knowledge of the framework.

https://gitlab.gnome.org/GNOME/gtk/issues/233

LockAndLol• 7 years ago

That would most certainly be abused by companies looking for free labor.

ehsankia2• 7 years ago

To be fair, it was a great nerd snipe, as it was a mysterious bug that happened when things aligned perfectly, yet still had a very good repro making it easy to test.

AdmiralAsshat• 7 years ago

Soon: "Hacker News Code Review As A Service"

Cthulhu_3• 7 years ago

Didn't StackOverflow already have that?

jnwatson• 7 years ago

It is hardly just "someone's personal repo". coveragepy has 1k github stars and is used by nearly every Python project under the sun.

hermitdev2• 7 years ago

Ive been a Python (among other languages) dev for about 25 years and I've never heard about this project until just now.

zaptheimpaler• 7 years ago

MechanicalHNTurk

the_jeremy• 7 years ago

I don't know about "personal repo". My employer uses this tool to help test production code.

alpb2• 7 years ago

Yeah that’s how many personal repos work. I assume you don’t use Node.js or Go or Python where most nontrivial dependencies often come from non-corporate third parties.

sefk• 7 years ago

Part of the reason this got a good response is due to Ned himself. nedbat@ has a great reputation in several communities. Also he showed his homework. And then posted with a good, tight repro case.

Fun to see the problem and the solution. Thanks Ned and thanks HN.

Source: have seen nedbat@ in action.

julienfr112• 7 years ago

That's not funny, that's the community being nice ! I don't want to live in a world where being nice is so rare that it ends up being funny.

pjc50• 7 years ago

It's the kind of drive that made stackoverflow work.

ehsankia2• 7 years ago

So the final answer is basically, there's nothing the author can do. The problem was half with the user's code mocking with internals, and half with downstream library not cleaning up properly.

gowld• 7 years ago

Mocks are terrible and should almost never be used, but this wasn't a mock. This was a monkeypatch.

Does anyone expect that their software would function properly in the face of arbitrary changes patched in by untrusted third parties ?

kortilla• 7 years ago

Monkey patching is a key feature of the python mock library. Get used to it if you’re getting into python development because you’re going to come across it.

hinkley• 7 years ago

The first really bizarre bug I fixed was a file descriptor bug. The code was streaming data and in some cases the app would just stop processing network packets. He or I figured out that keyboard input would temporarily unblock things.

He had a compound conditional without enough parentheses in it, and one of the possible outcomes resulted in fd=1, which is stdin on many operating systems. My general advice now is to assume a 5th grade reading level for math (which consists of +-x/), and use parentheses for everything else. Especially if you think yourself to be clever (narrator: he did).

nneonneo• 7 years ago

The very low precedence of & is the source of an unbelievable number of bugs, especially in embedded programming. For example:

if(PORTA & 3 != 0)

looks like it should be true if either of the low two bits is set. But actually, it only fires if the low bit is set because this resolves to PORTA & (3 != 0), which is PORTA & 1.

Some C compilers will warn you about this. Embedded C compilers are not usually the sort of compilers that are nice enough to have these warnings.

As a bonus, different languages handle precedence of &,|,^ differently: Python, for instance, binds & more tightly than ==.

Dylan168077• 7 years ago

It's a shame. C made the precedence blatantly wrong so a few thousand lines of legacy code wouldn't have to be updated.

And then everyone decides their own languages will copy it forever.

Cthulhu_3• 7 years ago

That should give a warning at least because it (and this is probably a C thing) treats or implicitly casts a boolean true / false to a 0/1. You shouldn't be able to do PORTA & true.

One more for my list of "why I don't like C".

hermitdev2• 7 years ago

I don't contribute much to open source, mostly due to employment agreements, but a couple I was able to make: 1. Bulk copying null values with Python Sybase module (initialized variable used) 2. ACE high res timer bug under x64. Inline ASM was using the wrong registers in x64 mode (reading garbage upper 32 bits of 64 bit value).

Both of these were bizarre bugs, and neither was immediately evident.

#1 was presented by an error saying something to the effect of "received xXxX bytes, expecting yyyy".

#2 was harder. It became evident because of logging. The high res timer was used gor logging amd intermittently, I was seeing +200 years supposedly elapsed during a db operation. Obviously wrong. It was a bug due to how registers are handled between x86 and x64 code.

Cthulhu_3• 7 years ago

Especially if you think yourself to be clever (narrator: he did).

"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it."

        — Brian W. Kernighan and P. J. Plauger in The Elements of Programming Style.

yhoiseth• 7 years ago

This made me laugh out loud:

Hacker News can be helpful, in spite of the tangents about shell redirection, authorship attribution, and GitHub monoculture.

kjksf• 7 years ago

This bug, the HN thread and how hard it is to fix (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• 7 years ago

Popcorn time...

deanCommie• 7 years ago

An 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• 7 years ago

Exceptions as flow-control is normal in Python. It’s used all over the place, e.g. generators with StopIteration.

sicromoft• 7 years ago

Terminology nit: Flow control is not the same as control flow (which is sometimes referred to as flow of control... yeah it's confusing).

https://en.wikipedia.org/wiki/Flow_control_(data)

https://en.wikipedia.org/wiki/Control_flow

masklinn8• 7 years ago

NumberFormatException is my bane in Java.

Surely that’s because of java? conversions normally throw in python and that’s no issue. So does indexing. So does str.index.

Dylan168077• 7 years ago

OP doesn't mention anything about KeyboardInterrupt

Please go check on the original post.

simion3144• 7 years ago

I 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.

masklinn8• 7 years ago

In 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.

rocqua2• 7 years ago

You could get a very similar issue in C++ where a destructor closes an FD that was already erroneously closed by an earlier call to a member function due to an exception.

The issue here is a bug in resource management. The specific bug was caused by an exception that shouldn't be caught still being caught. Which erroneously messed with resource management.

joshuamorton2• 7 years 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.

incompatible2• 7 years ago

Even C can map the returns values -1 through -50 to symbolic constants, typically defined in preprocessor headers.

ken• 7 years 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.

winstonewert5• 7 years ago

My 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• 7 years 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.

Dylan168077• 7 years 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.

thaumasiotes5• 7 years 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.

depressedpanda• 7 years ago

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:

Or, you can create a second null-like type in the language, name it 'undefined' perhaps, and return that when the key doesn't exist.

Of course, that may be considered kicking the can down the road as now you will have to trust developers to be competent enough to never store 'undefined' in the hash table, which I suppose is the reason you didn't mention JavaScript's solution to the problem.

masklinn8• 7 years ago

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.

Negative indices are valid in python. Also str.index raises.

And product types are not a good way to signal exclusive conditions.

mixmastamyk2• 7 years ago

A 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.

gigatexal• 7 years ago

2020 is the year of good vibes so far. The cool story of the reader getting hired from his experiences being featured on HN and now this.

Bug #915: Solved

Related Stories