Commit ec3c8f79 authored by Alexey Spiridonov's avatar Alexey Spiridonov Committed by Praveen Kumar Ramakrishnan

Destructor DCHECKs for EBADF

Summary: This almost always indicates a double-close bug, or a similarly nasty logic error. I don't dare make it a check, we may have some code running in production which would be broken by the CHECK, but this should give us early warning of any such bugs.

Test Plan:
```
fbconfig folly/test:file_test folly/test:file_util_test
fbmake runtests && fbmake runtests_opt
```

This test used to pass until @tudorb made me remove it due to worries about death tests messing with (currently absent) multi-threading:

```
+
+void testDoubleClose() {
+  File f("/dev/null");
+  checkUnixError(close(f.fd()));  // This feels so... wrong!
+  // The destructor will now try to double-close.
+}
+
+TEST(File, DCHECKDoubleClose) {
+#ifndef NDEBUG
+  // This test makes no sense otherwise, since this is a DCHECK.
+  EXPECT_DEATH(testDoubleClose(), "double-close-FD");
+#else
+  // That sound you hear is millions of lemmings falling to their doom.
+  testDoubleClose();
+#endif
+}
```

Reviewed By: tudorb@fb.com

Subscribers: folly-diffs@, yfeldblum, chalfant

FB internal diff: D2055610

Signature: t1:2055610:1431048270:a469d5c1f8182ffb74700908faa022e9613ed383
parent be171a18
......@@ -65,7 +65,11 @@ File& File::operator=(File&& other) {
}
File::~File() {
closeNoThrow(); // ignore error
auto fd = fd_;
if (!closeNoThrow()) { // ignore most errors
DCHECK_NE(errno, EBADF) << "closing fd " << fd << ", it may already "
<< "have been closed. Another time, this might close the wrong FD.";
}
}
/* static */ File File::temporary() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment