Live data from Hacker News

Bug #915: Solved

nedbatchelder.com

91–100 of 111 posts

Re: Bug #915: Solved

#91
post #5
post #4

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.

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.

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

Re: Bug #915: Solved

#92
post #89
post #86

Earlier quoted context omitted.

My impression is the opposite: random users demanding that some devs spend their time on a tricky corner issue and getting angry about it. After reading their comments, I'd be very hard pressed to motivate myself to work on it.

Well one person actually provided a solution, but then one of the maintainers just dismissed it because it was on Github, then locked the issue.

It seemed unclear that the patch addressed all the issues raised in the ticket, and looking at random patches still requires dev time. It is a different story when someone else actively works on getting it upstreamed and working on resolving any issues. It's about who volunteers to do the hard work.

Given that no one stepped up to do it yet and random users chimed in with aggressive and/or unhelpful noise, locking the issue was adequate.

I find it concerning how much abuse GNOME devs get, and I can completely understand that they respond like this.

Re: Bug #915: Solved

#93

Earlier quoted context omitted.

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

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

Time to climb out of your hole, Mr. Hermit!

Seriously though, we can't know every single third-party package under the sun. But this one is more important than the average.

Re: Bug #915: Solved

#94
post #65
post #23

godawful title. I refuse to read this. put a fucking description of what software project this is.

It's a followup to a story that got nearly 600 votes two days ago and spent a long time on the front page, so most of us are well aware of the context, but which project it is is beside the point. The bug was interesting and the debugging and conversation about it was interesting. https://news.ycombinator.com/item?id=22028581

that first title is also bad

Re: Bug #915: Solved

#95
post #11

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…

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.

Re: Bug #915: Solved

#96
post #22

Earlier quoted context omitted.

> Exceptions as flow-control is normal in Python. Perhaps it's high time that problem was solved?

This isn't a problem, it is a design decision that you evidently don't like. There is a difference.

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

Re: Bug #915: Solved

#97

Earlier quoted context omitted.

> This isn't really different from the multiple values approach. If you ignore most of what I said then it’s not. > Is that multiple values? (Yes.) Or a wrapper type? (Also yes, it's a tuple, a collection of values.) It’s neither type-safe nor unambiguous. > This works For low value of works. Which I guess is all you can ask for in a dynamically typed langage (eg Erlang uses this pattern to fairly good effect, but it…

> It’s neither type-safe nor unambiguous. How'd you get here? It's exactly as type-safe as the unwrapped hash table, which is admittedly not especially type-safe, and it's fully unambiguous.

> It's exactly as type-safe as the unwrapped hash table, which is admittedly not especially type-safe

Yes.

> and it's fully unambiguous.

No, you have any random value flagged as not present (in fact you can forget about or ignore the flag entirely), and no clue if that means anything.

Re: Bug #915: Solved

#98

Earlier quoted context omitted.

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.

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

> 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)

Re: Bug #915: Solved

#99

Earlier quoted context omitted.

> ("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…

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

Absolutely, but dealing with categories of errors is much harder with unstructured error codes.

Re: Bug #915: Solved

#100

Earlier quoted context omitted.

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

> 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, espec…

Thanks for spending the time explaining the problematic code.
Post reply on HN