src/base/Makefile.am | 3 +
src/base/file_descriptor.cc | 27 +++++++++++++++++
src/base/file_descriptor.h | 27 +++++++++++++++++
src/base/tests/unix_socket_test.cc | 60 +++++++++++++++++++++++++++++++++----
src/base/unix_client_socket.cc | 10 +-----
src/base/unix_server_socket.cc | 12 ++++++-
src/base/unix_socket.cc | 32 +++++++++++++------
src/base/unix_socket.h | 49 ++++++++++++++++++++----------
8 files changed, 177 insertions(+), 43 deletions(-)
Multiple issues have been fixed in the UnixSocket class:
* The UnixServerSocket failed if the named already exists in the file system.
* The UnixSocket used a mutex to protect Open() and Close(), but was is not
enough to make it thread safe. The mutex has been removed and users are now
responsible for protecting the instance if needed.
* The fd() method was not so useful since it required a message to be sent or
received before it returned a valid file descriptor. The fd() method now opens
the socket if it is not already open.
* The UnixClientSocket implemented a wait-loop in case connect() returns
EINPROGRESS. This has been changed to use a blocking call to connect()
instead.
diff --git a/src/base/Makefile.am b/src/base/Makefile.am
--- a/src/base/Makefile.am
+++ b/src/base/Makefile.am
@@ -32,6 +32,7 @@ lib_libopensaf_core_la_LDFLAGS += \
lib_libopensaf_core_la_SOURCES += \
src/base/condition_variable.cc \
src/base/daemon.c \
+ src/base/file_descriptor.cc \
src/base/file_notify.cc \
src/base/getenv.cc \
src/base/hj_dec.c \
@@ -76,6 +77,7 @@ noinst_HEADERS += \
src/base/buffer.h \
src/base/condition_variable.h \
src/base/daemon.h \
+ src/base/file_descriptor.h \
src/base/file_notify.h \
src/base/getenv.h \
src/base/log_message.h \
@@ -180,6 +182,7 @@ bin_libbase_test_LDFLAGS = \
src/base/lib_libopensaf_core_la-getenv.lo \
src/base/lib_libopensaf_core_la-log_message.lo \
src/base/lib_libopensaf_core_la-process.lo \
+ src/base/lib_libopensaf_core_la-file_descriptor.lo \
src/base/lib_libopensaf_core_la-file_notify.lo
bin_libbase_test_SOURCES = \
diff --git a/src/base/file_descriptor.cc b/src/base/file_descriptor.cc
new file mode 100644
--- /dev/null
+++ b/src/base/file_descriptor.cc
@@ -0,0 +1,27 @@
+/* -*- OpenSAF -*-
+ *
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
+ * under the GNU Lesser General Public License Version 2.1, February 1999.
+ * The complete license can be accessed from the following location:
+ * http://opensource.org/licenses/lgpl-license.php
+ * See the Copying file included with the OpenSAF distribution for full
+ * licensing terms.
+ *
+ */
+
+#include "base/file_descriptor.h"
+#include <fcntl.h>
+#include <unistd.h>
+
+namespace base {
+
+bool MakeFdNonblocking(int fd) {
+ int flags = fcntl(fd, F_GETFL);
+ return flags != -1 && fcntl(fd, F_SETFL, flags | O_NONBLOCK) != -1;
+}
+
+} // namespace base
diff --git a/src/base/file_descriptor.h b/src/base/file_descriptor.h
new file mode 100644
--- /dev/null
+++ b/src/base/file_descriptor.h
@@ -0,0 +1,27 @@
+/* -*- OpenSAF -*-
+ *
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed
+ * under the GNU Lesser General Public License Version 2.1, February 1999.
+ * The complete license can be accessed from the following location:
+ * http://opensource.org/licenses/lgpl-license.php
+ * See the Copying file included with the OpenSAF distribution for full
+ * licensing terms.
+ *
+ */
+
+#ifndef BASE_FILE_DESCRIPTOR_H_
+#define BASE_FILE_DESCRIPTOR_H_
+
+namespace base {
+
+// Set the a file descriptor's non-blocking flag. Returns true if successful,
+// and false otherwise.
+bool MakeFdNonblocking(int fd);
+
+} // namespace base
+
+#endif // BASE_FILE_DESCRIPTOR_H_
diff --git a/src/base/tests/unix_socket_test.cc
b/src/base/tests/unix_socket_test.cc
--- a/src/base/tests/unix_socket_test.cc
+++ b/src/base/tests/unix_socket_test.cc
@@ -1,6 +1,7 @@
/* -*- OpenSAF -*-
*
* (C) Copyright 2016 The OpenSAF Foundation
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
*
* This program is distributed in the hope that it will be useful, but
* WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
@@ -15,18 +16,18 @@
*
*/
+#include <sys/socket.h>
#include <sys/stat.h>
#include <unistd.h>
+#include <cerrno>
#include <cstdlib>
+#include <cstring>
#include <string>
#include "base/unix_server_socket.h"
#include "gtest/gtest.h"
class UnixSocketTest : public ::testing::Test {
- public:
-
protected:
-
UnixSocketTest() {
}
@@ -43,7 +44,6 @@ class UnixSocketTest : public ::testing:
return result == 0 && S_ISSOCK(buf.st_mode);
}
- // cppcheck-suppress unusedFunction
virtual void SetUp() {
char dir_tmpl[] = "/tmp/unix_socket_testXXXXXX";
char* dir_name = mkdtemp(dir_tmpl);
@@ -51,7 +51,6 @@ class UnixSocketTest : public ::testing:
tmpdir_ = std::string{dir_name};
}
- // cppcheck-suppress unusedFunction
virtual void TearDown() {
if (!tmpdir_.empty()) {
unlink(SocketName().c_str());
@@ -62,8 +61,8 @@ class UnixSocketTest : public ::testing:
std::string tmpdir_;
};
-TEST_F(UnixSocketTest, CheckThatServerUnlinksSocket) {
- EXPECT_FALSE(SocketExists());
+TEST_F(UnixSocketTest, DestructorShallUnlinkSocket) {
+ ASSERT_FALSE(SocketExists());
base::UnixServerSocket* sock = new base::UnixServerSocket(SocketName());
EXPECT_FALSE(SocketExists());
char buf[32];
@@ -72,3 +71,50 @@ TEST_F(UnixSocketTest, CheckThatServerUn
delete sock;
EXPECT_FALSE(SocketExists());
}
+
+TEST_F(UnixSocketTest, OpenShallUnlinkExistingSocket) {
+ ASSERT_FALSE(SocketExists());
+ int sock = socket(AF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC, 0);
+ ASSERT_GE(sock, 0);
+ struct sockaddr_un addr{AF_UNIX, {}};
+ memcpy(addr.sun_path, SocketName().c_str(), SocketName().size() + 1);
+ ASSERT_EQ(bind(sock,
+ reinterpret_cast<const struct sockaddr*>(&addr),
+ sizeof(addr)), 0);
+ ASSERT_EQ(close(sock), 0);
+ EXPECT_TRUE(SocketExists());
+ base::UnixServerSocket* server = new base::UnixServerSocket(SocketName());
+ char buf[32];
+ ssize_t result = server->Recv(buf, sizeof(buf));
+ int saved_errno = errno;
+ EXPECT_EQ(result, -1);
+ EXPECT_EQ(saved_errno, EAGAIN);
+ EXPECT_GE(server->fd(), 0);
+ delete server;
+}
+
+TEST_F(UnixSocketTest, DontUnlinkExistingSocketIfServerStillAlive) {
+ ASSERT_FALSE(SocketExists());
+ base::UnixServerSocket* sock = new base::UnixServerSocket(SocketName());
+ char buf[32];
+ ssize_t result = sock->Recv(buf, sizeof(buf));
+ EXPECT_EQ(result, -1);
+ EXPECT_EQ(errno, EAGAIN);
+ base::UnixServerSocket* sock2 = new base::UnixServerSocket(SocketName());
+ result = sock2->Recv(buf, sizeof(buf));
+ EXPECT_EQ(result, -1);
+ EXPECT_EQ(errno, EADDRINUSE);
+ delete sock2;
+ EXPECT_TRUE(SocketExists());
+ delete sock;
+ EXPECT_FALSE(SocketExists());
+}
+
+TEST_F(UnixSocketTest, FdMethodShallOpenTheSocket) {
+ ASSERT_FALSE(SocketExists());
+ base::UnixServerSocket* sock = new base::UnixServerSocket(SocketName());
+ int fd = sock->fd();
+ EXPECT_GE(fd, 0);
+ EXPECT_TRUE(SocketExists());
+ delete sock;
+}
diff --git a/src/base/unix_client_socket.cc b/src/base/unix_client_socket.cc
--- a/src/base/unix_client_socket.cc
+++ b/src/base/unix_client_socket.cc
@@ -1,6 +1,7 @@
/* -*- OpenSAF -*-
*
* (C) Copyright 2016 The OpenSAF Foundation
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
*
* This program is distributed in the hope that it will be useful, but
* WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
@@ -17,7 +18,6 @@
#include "base/unix_client_socket.h"
#include <sys/socket.h>
-#include <time.h>
#include <cerrno>
namespace base {
@@ -31,15 +31,9 @@ UnixClientSocket::~UnixClientSocket() {
bool UnixClientSocket::OpenHook(int sock) {
int result;
- int e;
do {
result = connect(sock, addr(), addrlen());
- e = errno;
- if (result != 0 && (e == EALREADY || e == EINPROGRESS)) {
- struct timespec delay{0, 10000000};
- clock_nanosleep(CLOCK_MONOTONIC, 0, &delay, nullptr);
- }
- } while (result != 0 && (e == EINTR || e == EALREADY || e == EINPROGRESS));
+ } while (result != 0 && errno == EINTR);
return result == 0;
}
diff --git a/src/base/unix_server_socket.cc b/src/base/unix_server_socket.cc
--- a/src/base/unix_server_socket.cc
+++ b/src/base/unix_server_socket.cc
@@ -1,6 +1,7 @@
/* -*- OpenSAF -*-
*
* (C) Copyright 2016 The OpenSAF Foundation
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
*
* This program is distributed in the hope that it will be useful, but
* WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
@@ -27,10 +28,19 @@ UnixServerSocket::UnixServerSocket(const
}
UnixServerSocket::~UnixServerSocket() {
- if (fd() >= 0) UnixServerSocket::CloseHook();
+ if (get_fd() >= 0) UnixServerSocket::CloseHook();
}
bool UnixServerSocket::OpenHook(int sock) {
+ int tmp_sock = socket(AF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC, 0);
+ int connect_result;
+ int connect_errno;
+ do {
+ connect_result = connect(tmp_sock, addr(), addrlen());
+ connect_errno = errno;
+ } while (connect_result != 0 && connect_errno == EINTR);
+ close(tmp_sock);
+ if (connect_result != 0 && connect_errno == ECONNREFUSED) unlink(path());
return bind(sock, addr(), addrlen()) == 0;
}
diff --git a/src/base/unix_socket.cc b/src/base/unix_socket.cc
--- a/src/base/unix_socket.cc
+++ b/src/base/unix_socket.cc
@@ -1,6 +1,7 @@
/* -*- OpenSAF -*-
*
* (C) Copyright 2016 The OpenSAF Foundation
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
*
* This program is distributed in the hope that it will be useful, but
* WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
@@ -19,14 +20,17 @@
#include <sys/socket.h>
#include <unistd.h>
#include <cstring>
+#include "base/file_descriptor.h"
#include "base/osaf_utility.h"
+#include "base/time.h"
namespace base {
UnixSocket::UnixSocket(const std::string& path) :
fd_{-1},
addr_{AF_UNIX, {}},
- mutex_{} {
+ last_failed_open_{},
+ saved_errno_{} {
if (path.size() < sizeof(addr_.sun_path)) {
memcpy(addr_.sun_path, path.c_str(), path.size() + 1);
} else {
@@ -35,18 +39,27 @@ UnixSocket::UnixSocket(const std::string
}
int UnixSocket::Open() {
- Lock lock(mutex_);
int sock = fd_;
if (sock < 0) {
if (addr_.sun_path[0] != '\0') {
- sock = socket(AF_UNIX, SOCK_DGRAM | SOCK_NONBLOCK | SOCK_CLOEXEC, 0);
- if (sock >= 0 && !OpenHook(sock)) {
- int e = errno;
- close(sock);
- sock = -1;
- errno = e;
+ struct timespec current_time = ReadMonotonicClock();
+ if (TimespecToMillis(current_time - last_failed_open_) != 0) {
+ sock = socket(AF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC, 0);
+ if (sock >= 0 && !OpenHook(sock)) {
+ int e = errno;
+ close(sock);
+ sock = -1;
+ errno = e;
+ }
+ fd_ = sock;
+ if (sock >=0 && MakeFdNonblocking(sock) == false) Close();
+ if (fd_ < 0) {
+ last_failed_open_ = current_time;
+ saved_errno_ = errno;
+ }
+ } else {
+ errno = saved_errno_;
}
- fd_ = sock;
} else {
errno = ENAMETOOLONG;
}
@@ -61,7 +74,6 @@ UnixSocket::~UnixSocket() {
}
void UnixSocket::Close() {
- Lock lock(mutex_);
int sock = fd_;
if (sock >= 0) {
int e = errno;
diff --git a/src/base/unix_socket.h b/src/base/unix_socket.h
--- a/src/base/unix_socket.h
+++ b/src/base/unix_socket.h
@@ -1,6 +1,7 @@
/* -*- OpenSAF -*-
*
* (C) Copyright 2016 The OpenSAF Foundation
+ * Copyright Ericsson AB 2017 - All Rights Reserved.
*
* This program is distributed in the hope that it will be useful, but
* WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
@@ -21,25 +22,40 @@
#include <pthread.h>
#include <sys/socket.h>
#include <sys/un.h>
+#include <time.h>
#include <cerrno>
#include <string>
#include "base/macros.h"
-#include "base/mutex.h"
namespace base {
-// A class implementing non-blocking operations on a UNIX domain socket.
+// A class implementing non-blocking operations on a UNIX domain socket. This
+// class is not thread-safe and must be protected by a mutex if shared between
+// multiple threads.
class UnixSocket {
public:
// Close the socket.
virtual ~UnixSocket();
+ // Returns the current file descriptor for this UNIX socket, or -1 if the
+ // socket could not be opened. Note that the Send() and Recv() methods may
+ // open and/or close the socket, and the file descriptor can therefore
+ // potentially be change as a result of calling any of these two methods. If
a
+ // UnixSocket instance is shared between multiple threads (and therefore
+ // protected with a mutex), this means that the result from the fd() method
is
+ // only valid for as long as the mutex protecting the instance is held.
+ int fd() {
+ int sock = fd_;
+ if (sock < 0) sock = Open();
+ return sock;
+ }
// Send a message in non-blocking mode. This call will open the socket if it
// was not already open. The EINTR error code from the send() libc function
is
- // handled by retrying the send() call in a loop. In case of other errors,
the
- // socket will be closed.
+ // handled by retrying the send() call in a loop. In case of other errors, -1
+ // is returned and errno contains one of the codes listed in the recv() libc
+ // function. The socket will be closed in case the error was not EINTR,
EAGAIN
+ // or EWOULDBLOCK.
ssize_t Send(const void* buffer, size_t length) {
- int sock = fd_;
- if (sock < 0) sock = Open();
+ int sock = fd();
ssize_t result = -1;
if (sock >= 0) {
do {
@@ -52,10 +68,11 @@ class UnixSocket {
// Receive a message in non-blocking mode. This call will open the socket if
// it was not already open. The EINTR error code from the recv() libc
function
// is handled by retrying the recv() call in a loop. In case of other errors,
- // the socket will be closed.
+ // -1 is returned and errno contains one of the codes listed in the recv()
+ // libc function. The socket will be closed in case the error was not EINTR,
+ // EAGAIN or EWOULDBLOCK.
ssize_t Recv(void* buffer, size_t length) {
- int sock = fd_;
- if (sock < 0) sock = Open();
+ int sock = fd();
ssize_t result = -1;
if (sock >= 0) {
do {
@@ -65,28 +82,26 @@ class UnixSocket {
}
return result;
}
- // Returns the current file descriptor for this UNIX socket, or -1 if the
- // socket is currently not open. Note that the Send() and Recv() methods may
- // open and/or close the socket, and potentially the file descriptor will be
- // different after a call to any of these two methods.
- int fd() const { return fd_; }
protected:
explicit UnixSocket(const std::string& path);
- int Open();
- void Close();
virtual bool OpenHook(int sock);
virtual void CloseHook();
const struct sockaddr* addr() const {
return reinterpret_cast<const struct sockaddr*>(&addr_);
}
static socklen_t addrlen() { return sizeof(addr_); }
+ int get_fd() const { return fd_; }
const char* path() const { return addr_.sun_path; }
private:
+ int Open();
+ void Close();
+
int fd_;
struct sockaddr_un addr_;
- Mutex mutex_;
+ struct timespec last_failed_open_;
+ int saved_errno_;
DELETE_COPY_AND_MOVE_OPERATORS(UnixSocket);
};
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
