5 ms·

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

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.

masklinn 7y 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.
- simion314 7y ago
  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).
  - 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)
    - simion314 7y ago
      Thanks for spending the time explaining the problematic code.
rocqua 7y 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.
- simion314 7y ago
  I did not see that in the snippets I will try to find the section, was the bug in the code that was catching the exception or it was thrown by mistake. I agree that is a resource management issue and you often make this kind of bugs when you try to reuse objects (for performance reasons most of the time)

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.

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

simion314 7y ago
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).
- 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)
  - simion314 7y ago
    Thanks for spending the time explaining the problematic code.

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

simion314 7y ago
I did not see that in the snippets I will try to find the section, was the bug in the code that was catching the exception or it was thrown by mistake. I agree that is a resource management issue and you often make this kind of bugs when you try to reuse objects (for performance reasons most of the time)