Github user ben-craig commented on the pull request:
https://github.com/apache/thrift/pull/142#issuecomment-46216062
Closing invalid file descriptors isn't safe. You could be closing someone
else's file descriptor out from underneath them. Seems like some investigation
is needed to figure out why the isOpen() check isn't preventing double-close
situations.
That being said... the old code wasn't exactly safe either.
One other note that doesn't need to be solved by you, but it would be
awesome if you could file a JIRA bug about it...
The TTransportTest has some tests for TFDTransport and TFileTransport, and
they are #ifdef'ed out on Windows. Ideally, they wouldn't be #ifdef'ed out
after this change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
