lawrence_danna added a comment. In D69488#1723481 <https://reviews.llvm.org/D69488#1723481>, @labath wrote:
> I am not thrilled by all of that duping going around. Having multiple FILE > objects means that you have multiple independent file caches too. That can > cause different kinds of strange behavior if multiple things start > reading/writing to the different FILE objects simultaneously. I think I'd > rather just keep the existing borrow semantics. I don't think that should be > a problem in practice -- it's just that this test is weird because is testing > the extreme cases of this behavior (which is fine). Normally you'll just use > one level of wrapping and so the underlying file will be naturally kept > around, since lldb will still be holding onto it. I'm not exactly thrilled with it but I will still argue it is the right thing to do. If you look at the docstring for `GetFile`, it doesn't mention this problem. It says you can't use a borrowed file if it becomes dangling, but it doesn't say you have to delete your references to it before it can dangle. That's because this problem is surprising enough that neither of us thought of it. Consider this situation: The python programmer makes an object that keeps a reference to `debugger` and also to `debugger.GetOutputFile().GetFile()`. When they're done with the object they destroy the debugger and let their object go out of scope. In python3, they may get an `IOError` at this point, as the saved output file tries to flush its buffer to a closed file. In python 2 they are exposed to to a use-after free violation and the program could crash! It is very difficult to write python programs that deal with these kind of semantics correctly, and nothing about this API suggests that it is as dangerous to use as that. I think the correctness and ergonomics concerns outweigh the performance concerns here. Particularly because this issue only affects python 2. Is it really a bigger deal that we generate some extra dups on an end-of-life scripting language than that we potentially crash on that language? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69488/new/ https://reviews.llvm.org/D69488 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
