face: improve error handling in UnixStreamChannel
Change-Id: I5d37b56e74264490089a4c18f527bbcc202a480d
diff --git a/daemon/face/unix-stream-channel.cpp b/daemon/face/unix-stream-channel.cpp
index 150fad4..a1abe9c 100644
--- a/daemon/face/unix-stream-channel.cpp
+++ b/daemon/face/unix-stream-channel.cpp
@@ -30,7 +30,6 @@
#include "common/global.hpp"
#include <boost/filesystem.hpp>
-#include <sys/stat.h> // for chmod()
namespace nfd::face {
@@ -50,10 +49,10 @@
{
if (isListening()) {
// use the non-throwing variants during destruction and ignore any errors
- boost::system::error_code error;
- m_acceptor.close(error);
+ boost::system::error_code ec;
+ m_acceptor.close(ec);
NFD_LOG_CHAN_TRACE("Removing socket file");
- boost::filesystem::remove(m_endpoint.path(), error);
+ boost::filesystem::remove(m_endpoint.path(), ec);
}
}
@@ -70,42 +69,56 @@
namespace fs = boost::filesystem;
fs::path socketPath = m_endpoint.path();
- fs::file_type type = fs::symlink_status(socketPath).type();
-
- if (type == fs::socket_file) {
- boost::system::error_code error;
- boost::asio::local::stream_protocol::socket socket(getGlobalIoService());
- socket.connect(m_endpoint, error);
- NFD_LOG_CHAN_TRACE("connect() on existing socket file returned: " << error.message());
- if (!error) {
- // someone answered, leave the socket alone
- NDN_THROW(Error("Socket file at " + m_endpoint.path() + " belongs to another process"));
- }
- else if (error == boost::asio::error::connection_refused ||
- error == boost::asio::error::timed_out) {
- // no one is listening on the remote side,
- // we can safely remove the stale socket
- NFD_LOG_CHAN_DEBUG("Removing stale socket file");
- fs::remove(socketPath);
- }
- }
- else if (type != fs::file_not_found) {
- NDN_THROW(Error(m_endpoint.path() + " already exists and is not a socket file"));
- }
-
- // ensure parent directory exists before creating socket
+ // ensure parent directory exists
fs::path parent = socketPath.parent_path();
if (!parent.empty() && fs::create_directories(parent)) {
NFD_LOG_CHAN_TRACE("Created directory " << parent);
}
- m_acceptor.open();
- m_acceptor.bind(m_endpoint);
- m_acceptor.listen(backlog);
-
- if (::chmod(m_endpoint.path().data(), 0666) < 0) {
- NDN_THROW_ERRNO(Error("Failed to chmod " + m_endpoint.path()));
+ boost::system::error_code ec;
+ fs::file_type type = fs::symlink_status(socketPath).type();
+ if (type == fs::socket_file) {
+ // if the socket file already exists, there may be another instance
+ // of NFD running on the system: make sure we don't steal its socket
+ boost::asio::local::stream_protocol::socket socket(getGlobalIoService());
+ socket.connect(m_endpoint, ec);
+ NFD_LOG_CHAN_TRACE("connect() on existing socket file returned: " << ec.message());
+ if (!ec) {
+ // someone answered, leave the socket alone
+ ec = boost::system::errc::make_error_code(boost::system::errc::address_in_use);
+ NDN_THROW_NO_STACK(fs::filesystem_error("UnixStreamChannel::listen", socketPath, ec));
+ }
+ else if (ec == boost::asio::error::connection_refused ||
+ ec == boost::asio::error::timed_out) {
+ // no one is listening on the remote side, we can safely remove the stale socket
+ NFD_LOG_CHAN_DEBUG("Removing stale socket file");
+ fs::remove(socketPath);
+ }
}
+ else if (type != fs::file_not_found) {
+ // the file exists but is not a socket: this is a fatal error as we cannot
+ // safely overwrite the file without potentially risking data loss
+ ec = boost::system::errc::make_error_code(boost::system::errc::not_a_socket);
+ NDN_THROW_NO_STACK(fs::filesystem_error("UnixStreamChannel::listen", socketPath, ec));
+ }
+
+ try {
+ m_acceptor.open();
+ m_acceptor.bind(m_endpoint);
+ m_acceptor.listen(backlog);
+ }
+ catch (const boost::system::system_error& e) {
+ // exceptions thrown by Boost.Asio are very terse, add more context
+ NDN_THROW_NO_STACK(fs::filesystem_error("UnixStreamChannel::listen: "s + e.std::runtime_error::what(),
+ socketPath, e.code()));
+ }
+
+ // do this here so that, even if the calls below fail,
+ // the destructor will still remove the socket file
+ m_isListening = true;
+
+ fs::permissions(socketPath, fs::owner_read | fs::group_read | fs::others_read |
+ fs::owner_write | fs::group_write | fs::others_write);
accept(onFaceCreated, onAcceptFailed);
NFD_LOG_CHAN_DEBUG("Started listening");
diff --git a/daemon/face/unix-stream-channel.hpp b/daemon/face/unix-stream-channel.hpp
index 7c7f287..dbe53d4 100644
--- a/daemon/face/unix-stream-channel.hpp
+++ b/daemon/face/unix-stream-channel.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2023, Regents of the University of California,
+ * Copyright (c) 2014-2024, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -46,15 +46,6 @@
{
public:
/**
- * \brief UnixStreamChannel-related error.
- */
- class Error : public std::runtime_error
- {
- public:
- using std::runtime_error::runtime_error;
- };
-
- /**
* \brief Create a UnixStream channel for the specified \p endpoint.
*
* To enable the creation of faces upon incoming connections, one needs to
@@ -67,7 +58,7 @@
bool
isListening() const final
{
- return m_acceptor.is_open();
+ return m_isListening;
}
size_t
@@ -89,7 +80,7 @@
* returns an error)
* \param backlog The maximum length of the queue of pending incoming
* connections
- * \throw Error
+ * \throw boost::system::system_error
*/
void
listen(const FaceCreatedCallback& onFaceCreated,
@@ -104,6 +95,7 @@
private:
const unix_stream::Endpoint m_endpoint;
const bool m_wantCongestionMarking;
+ bool m_isListening = false;
boost::asio::local::stream_protocol::acceptor m_acceptor;
size_t m_size = 0;
};
diff --git a/daemon/main.cpp b/daemon/main.cpp
index bfaba73..725b7b6 100644
--- a/daemon/main.cpp
+++ b/daemon/main.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2023, Regents of the University of California,
+ * Copyright (c) 2014-2024, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -36,10 +36,10 @@
#include <boost/asio/signal_set.hpp>
#include <boost/config.hpp>
#include <boost/exception/diagnostic_information.hpp>
-#include <boost/filesystem.hpp>
#include <boost/program_options/options_description.hpp>
#include <boost/program_options/parsers.hpp>
#include <boost/program_options/variables_map.hpp>
+#include <boost/system/system_error.hpp>
#include <boost/version.hpp>
#include <atomic>
@@ -317,13 +317,19 @@
", with ndn-cxx version " NDN_CXX_VERSION_BUILD_STRING
<< std::endl;
+ auto flush = ndn::make_scope_exit([] { ndn::util::Logging::flush(); });
+
NfdRunner runner(configFile);
try {
runner.initialize();
}
- catch (const boost::filesystem::filesystem_error& e) {
+ catch (const boost::system::system_error& e) {
NFD_LOG_FATAL(boost::diagnostic_information(e));
- return e.code() == boost::system::errc::permission_denied ? 4 : 1;
+ if (e.code() == boost::system::errc::operation_not_permitted ||
+ e.code() == boost::system::errc::permission_denied) {
+ return 4;
+ }
+ return 1;
}
catch (const std::exception& e) {
NFD_LOG_FATAL(boost::diagnostic_information(e));