[fs] Ensure Vnode::Close is called on unclean teardown
The contract of ulib/fs is that every call to "Open"
will be paired with an accompanying call to "Close",
in the event of either:
1) A client sends a close message explicitly, making
their channel defunct (and no longer observed), or
2) A client closes their channel, implicitly closing
their connection to the filesystem.
Although the Connection class in ulib/fs would properly
cancel waits and tear down the connection object,
it previoulsy missed sending a close in the second
condition, since it invoked "CallHandler" while the
channel was not yet ZX_HANDLE_INVALID.
This patch ensures that the close handler is invoked
in this condition.
ZX-1853 #done
Change-Id: I291192cae32e8250f7cef932cbf1a4de4b8b2d2d
diff --git a/system/ulib/fdio/remoteio.c b/system/ulib/fdio/remoteio.c
index 1234980..cff470d 100644
--- a/system/ulib/fdio/remoteio.c
+++ b/system/ulib/fdio/remoteio.c
@@ -109,7 +109,7 @@
msg.hcount = 0;
#endif
cb(&msg, cookie);
- return ZX_OK;
+ return ERR_DISPATCHER_DONE;
}
zx_status_t zxrio_handler(zx_handle_t h, zxrio_cb_t cb, void* cookie) {
diff --git a/system/ulib/fs/connection.cpp b/system/ulib/fs/connection.cpp
index ea79d3d..bbb6ba6 100644
--- a/system/ulib/fs/connection.cpp
+++ b/system/ulib/fs/connection.cpp
@@ -150,7 +150,7 @@
// Give the dispatcher a chance to clean up.
if (status != ERR_DISPATCHER_DONE) {
- CallHandler();
+ CallClose();
}
// Tell the VFS that the connection closed remotely.
@@ -167,7 +167,7 @@
ZX_DEBUG_ASSERT_MSG(status == ZX_OK, "Could not cancel wait: status=%d", status);
wait_.set_object(ZX_HANDLE_INVALID);
- CallHandler();
+ CallClose();
}
// Release the token associated with this connection's vnode since the connection
@@ -192,6 +192,11 @@
return zxrio_handler(channel_.get(), &Connection::HandleMessageThunk, this);
}
+void Connection::CallClose() {
+ channel_.reset();
+ CallHandler();
+}
+
zx_status_t Connection::HandleMessageThunk(zxrio_msg_t* msg, void* cookie) {
Connection* connection = static_cast<Connection*>(cookie);
return connection->HandleMessage(msg);
diff --git a/system/ulib/fs/include/fs/connection.h b/system/ulib/fs/include/fs/connection.h
index 5ac20a4..3a3e91a 100644
--- a/system/ulib/fs/include/fs/connection.h
+++ b/system/ulib/fs/include/fs/connection.h
@@ -57,6 +57,11 @@
private:
zx_status_t CallHandler();
+ // Sends an explicit close message to the underlying vnode.
+ // Only necessary if the handler has not returned ERR_DISPATCHER_DONE
+ // and has been opened.
+ void CallClose();
+
static zx_status_t HandleMessageThunk(zxrio_msg_t* msg, void* cookie);
zx_status_t HandleMessage(zxrio_msg_t* msg);
diff --git a/system/utest/fs/test-basic.c b/system/utest/fs/test-basic.c
index 300ca75..b8e2a11 100644
--- a/system/utest/fs/test-basic.c
+++ b/system/utest/fs/test-basic.c
@@ -9,7 +9,10 @@
#include <sys/stat.h>
#include <unistd.h>
+#include <fdio/limits.h>
+#include <fdio/util.h>
#include <unittest/unittest.h>
+#include <zircon/syscalls.h>
#include "filesystems.h"
@@ -68,6 +71,29 @@
END_TEST;
}
+bool test_unclean_close(void) {
+ BEGIN_TEST;
+
+ int fd = open("::foobar", O_CREAT | O_RDWR);
+ ASSERT_GT(fd, 0, "");
+
+ // Try closing a connection to a file with an "unclean" shutdown,
+ // noticed by the filesystem server as a closed handle rather than
+ // an explicit "Close" call.
+ zx_handle_t handles[FDIO_MAX_HANDLES];
+ uint32_t types[FDIO_MAX_HANDLES];
+ zx_status_t r = fdio_transfer_fd(fd, 0, handles, types);
+ ASSERT_GE(fd, 0, "");
+ for (size_t i = 0; i < (size_t) r; i++) {
+ ASSERT_EQ(zx_handle_close(handles[i]), ZX_OK, "");
+ }
+
+ ASSERT_EQ(unlink("::foobar"), 0, "");
+
+ END_TEST;
+}
+
RUN_FOR_ALL_FILESYSTEMS(basic_tests,
RUN_TEST_MEDIUM(test_basic)
+ RUN_TEST_MEDIUM(test_unclean_close)
)
