Merge pull request #2437 from BioDataAnalysis/bda_several_improvements
Several smaller improvements in the C++ code and build
diff --git a/build/cmake/BoostMacros.cmake b/build/cmake/BoostMacros.cmake
index cc50b53..ffb85af 100644
--- a/build/cmake/BoostMacros.cmake
+++ b/build/cmake/BoostMacros.cmake
@@ -19,17 +19,10 @@
set(BOOST_MINREV 1.56)
-# we are not ready for the new style link targets introduced in
-# boost 1.70.0 and cmake 3.14.2 which showed up on appveyor in
-# mingw builds
-set(Boost_NO_BOOST_CMAKE ON)
-
macro(REQUIRE_BOOST_HEADERS)
find_package(Boost ${BOOST_MINREV} QUIET REQUIRED)
if (NOT Boost_FOUND)
- string(CONCAT fatal_message
- "Boost ${BOOST_MINREV} or later is required to build sources in ${CMAKE_CURRENT_SOURCE_DIR}")
- message(FATAL_ERROR, ${fatal_message})
+ message(FATAL_ERROR "Boost ${BOOST_MINREV} or later is required to build sources in ${CMAKE_CURRENT_SOURCE_DIR}")
endif()
if (DEFINED Boost_INCLUDE_DIRS)
# pre-boost 1.70.0 aware cmake, otherwise it is using targets
@@ -41,9 +34,6 @@
message(STATUS "Locating boost libraries required by sources in ${CMAKE_CURRENT_SOURCE_DIR}")
find_package(Boost ${BOOST_MINREV} REQUIRED COMPONENTS ${${libs}})
if (NOT Boost_FOUND)
- string(CONCAT fatal_message
- "Boost ${BOOST_MINREV} or later is required to build sources in ${CMAKE_CURRENT_SOURCE_DIR}, "
- "or use -DBUILD_TESTING=OFF")
- message(FATAL_ERROR, ${fatal_message})
+ message(FATAL_ERROR "Boost ${BOOST_MINREV} or later is required to build sources in ${CMAKE_CURRENT_SOURCE_DIR}, or use -DBUILD_TESTING=OFF")
endif()
endmacro()
diff --git a/build/cmake/DefineOptions.cmake b/build/cmake/DefineOptions.cmake
index 1fa7a56..3cca31e 100644
--- a/build/cmake/DefineOptions.cmake
+++ b/build/cmake/DefineOptions.cmake
@@ -145,7 +145,7 @@
# Visual Studio only options
if(MSVC)
- option(WITH_MT "Build using MT instead of MD (MSVC only)" OFF)
+ option(WITH_MT "Build using the static runtime 'MT' instead of the shared DLL-specific runtime 'MD' (MSVC only)" OFF)
endif(MSVC)
macro(MESSAGE_DEP flag summary)
@@ -207,6 +207,9 @@
message(STATUS " Build Python library: ${BUILD_PYTHON}")
MESSAGE_DEP(WITH_PYTHON "Disabled by WITH_PYTHON=OFF")
MESSAGE_DEP(PYTHONLIBS_FOUND "Python libraries missing")
+if(MSVC)
+ message(STATUS " Using static runtime library: ${WITH_MT}")
+endif(MSVC)
message(STATUS)
message(STATUS)
message(STATUS "----------------------------------------------------------")
diff --git a/lib/cpp/src/thrift/protocol/TCompactProtocol.tcc b/lib/cpp/src/thrift/protocol/TCompactProtocol.tcc
index 1678091..9270ab8 100644
--- a/lib/cpp/src/thrift/protocol/TCompactProtocol.tcc
+++ b/lib/cpp/src/thrift/protocol/TCompactProtocol.tcc
@@ -20,6 +20,7 @@
#define _THRIFT_PROTOCOL_TCOMPACTPROTOCOL_TCC_ 1
#include <limits>
+#include <cstdlib>
#include "thrift/config.h"
diff --git a/lib/cpp/src/thrift/transport/TBufferTransports.h b/lib/cpp/src/thrift/transport/TBufferTransports.h
index 8f527bb..3ef8d1f 100644
--- a/lib/cpp/src/thrift/transport/TBufferTransports.h
+++ b/lib/cpp/src/thrift/transport/TBufferTransports.h
@@ -150,7 +150,7 @@
* performance-sensitive operation, so it is okay to just leave it to
* the concrete class to set up pointers correctly.
*/
- TBufferBase(std::shared_ptr<TConfiguration> config = nullptr)
+ TBufferBase(std::shared_ptr<TConfiguration> config = nullptr)
: TVirtualTransport(config), rBase_(nullptr), rBound_(nullptr), wBase_(nullptr), wBound_(nullptr) {}
/// Convenience mutator for setting the read buffer.
@@ -211,7 +211,7 @@
}
/// Use specified read and write buffer sizes.
- TBufferedTransport(std::shared_ptr<TTransport> transport, uint32_t rsz, uint32_t wsz,
+ TBufferedTransport(std::shared_ptr<TTransport> transport, uint32_t rsz, uint32_t wsz,
std::shared_ptr<TConfiguration> config = nullptr)
: TVirtualTransport(config),
transport_(transport),
@@ -506,7 +506,7 @@
* TAKE_OWNERSHIP:
* TMemoryBuffer will become the "owner" of the buffer,
* and will be responsible for freeing it.
- * The membory must have been allocated with malloc.
+ * The memory must have been allocated with malloc.
*/
enum MemoryPolicy { OBSERVE = 1, COPY = 2, TAKE_OWNERSHIP = 3 };
@@ -515,8 +515,8 @@
* owned by the TMemoryBuffer object.
*/
TMemoryBuffer(std::shared_ptr<TConfiguration> config = nullptr)
- : TVirtualTransport(config) {
- initCommon(nullptr, defaultSize, true, 0);
+ : TVirtualTransport(config) {
+ initCommon(nullptr, defaultSize, true, 0);
}
/**
@@ -525,9 +525,9 @@
*
* @param sz The initial size of the buffer.
*/
- TMemoryBuffer(uint32_t sz, std::shared_ptr<TConfiguration> config = nullptr)
- : TVirtualTransport(config) {
- initCommon(nullptr, sz, true, 0);
+ TMemoryBuffer(uint32_t sz, std::shared_ptr<TConfiguration> config = nullptr)
+ : TVirtualTransport(config) {
+ initCommon(nullptr, sz, true, 0);
}
/**
@@ -540,7 +540,7 @@
* @param sz The size of @c buf.
* @param policy See @link MemoryPolicy @endlink .
*/
- TMemoryBuffer(uint8_t* buf, uint32_t sz, MemoryPolicy policy = OBSERVE, std::shared_ptr<TConfiguration> config = nullptr)
+ TMemoryBuffer(uint8_t* buf, uint32_t sz, MemoryPolicy policy = OBSERVE, std::shared_ptr<TConfiguration> config = nullptr)
: TVirtualTransport(config) {
if (buf == nullptr && sz != 0) {
throw TTransportException(TTransportException::BAD_ARGS,
diff --git a/lib/cpp/src/thrift/transport/TServerSocket.cpp b/lib/cpp/src/thrift/transport/TServerSocket.cpp
index 5e7e2c0..671cabc 100644
--- a/lib/cpp/src/thrift/transport/TServerSocket.cpp
+++ b/lib/cpp/src/thrift/transport/TServerSocket.cpp
@@ -187,7 +187,7 @@
if (!listening_)
return false;
- if (!path_.empty() && (path_[0] != '\0')) {
+ if (isUnixDomainSocket() && (path_[0] != '\0')) {
// On some platforms the domain socket file may not be instantly
// available yet, i.e. the Windows file system can be slow. Therefore
// we should check that the domain socket file actually exists.
@@ -339,7 +339,7 @@
// Defer accept
#ifdef TCP_DEFER_ACCEPT
- if (path_.empty()) {
+ if (!isUnixDomainSocket()) {
if (-1 == setsockopt(serverSocket_, IPPROTO_TCP, TCP_DEFER_ACCEPT, &one, sizeof(one))) {
int errno_copy = THRIFT_GET_SOCKET_ERROR;
GlobalOutput.perror("TServerSocket::listen() setsockopt() TCP_DEFER_ACCEPT ", errno_copy);
@@ -391,8 +391,6 @@
= std::shared_ptr<THRIFT_SOCKET>(new THRIFT_SOCKET(sv[0]), destroyer_of_fine_sockets);
}
- // tcp == false means Unix Domain socket
- bool tcp = (path_.empty());
// Validate port number
if (port_ < 0 || port_ > 0xFFFF) {
@@ -401,7 +399,7 @@
// Resolve host:port strings into an iterable of struct addrinfo*
AddressResolutionHelper resolved_addresses;
- if (tcp) {
+ if (!isUnixDomainSocket()) {
try {
resolved_addresses.resolve(address_, std::to_string(port_), SOCK_STREAM,
#ifdef ANDROID
@@ -422,7 +420,7 @@
int retries = 0;
int errno_copy = 0;
- if (!tcp) {
+ if (isUnixDomainSocket()) {
// -- Unix Domain Socket -- //
serverSocket_ = socket(PF_UNIX, SOCK_STREAM, IPPROTO_IP);
@@ -538,7 +536,7 @@
// throw an error if we failed to bind properly
if (retries > retryLimit_) {
char errbuf[1024];
- if (!tcp) {
+ if (isUnixDomainSocket()) {
THRIFT_SNPRINTF(errbuf, sizeof(errbuf), "TServerSocket::listen() PATH %s", path_.c_str());
} else {
THRIFT_SNPRINTF(errbuf, sizeof(errbuf), "TServerSocket::listen() BIND %d", port_);
@@ -565,10 +563,18 @@
listening_ = true;
}
-int TServerSocket::getPort() {
+int TServerSocket::getPort() const {
return port_;
}
+std::string TServerSocket::getPath() const {
+ return path_;
+}
+
+bool TServerSocket::isUnixDomainSocket() const {
+ return !path_.empty();
+}
+
shared_ptr<TTransport> TServerSocket::acceptImpl() {
if (serverSocket_ == THRIFT_INVALID_SOCKET) {
throw TTransportException(TTransportException::NOT_OPEN, "TServerSocket not listening");
diff --git a/lib/cpp/src/thrift/transport/TServerSocket.h b/lib/cpp/src/thrift/transport/TServerSocket.h
index e4659a0..c87a7f6 100644
--- a/lib/cpp/src/thrift/transport/TServerSocket.h
+++ b/lib/cpp/src/thrift/transport/TServerSocket.h
@@ -125,7 +125,11 @@
THRIFT_SOCKET getSocketFD() override { return serverSocket_; }
- int getPort();
+ int getPort() const;
+
+ std::string getPath() const;
+
+ bool isUnixDomainSocket() const;
void listen() override;
void interrupt() override;
diff --git a/lib/cpp/src/thrift/transport/TSocket.cpp b/lib/cpp/src/thrift/transport/TSocket.cpp
index 2f0eb9c..1542c08 100644
--- a/lib/cpp/src/thrift/transport/TSocket.cpp
+++ b/lib/cpp/src/thrift/transport/TSocket.cpp
@@ -265,7 +265,7 @@
return;
}
- if (!path_.empty()) {
+ if (isUnixDomainSocket()) {
socket_ = socket(PF_UNIX, SOCK_STREAM, IPPROTO_IP);
} else {
socket_ = socket(res->ai_family, res->ai_socktype, res->ai_protocol);
@@ -330,7 +330,7 @@
// Connect the socket
int ret;
- if (!path_.empty()) {
+ if (isUnixDomainSocket()) {
/*
* TODO: seems that windows now support unix sockets,
@@ -408,7 +408,7 @@
throw TTransportException(TTransportException::NOT_OPEN, "THRIFT_FCNTL() failed", errno_copy);
}
- if (path_.empty()) {
+ if (!isUnixDomainSocket()) {
setCachedAddress(res->ai_addr, static_cast<socklen_t>(res->ai_addrlen));
}
}
@@ -417,7 +417,7 @@
if (isOpen()) {
return;
}
- if (!path_.empty()) {
+ if (isUnixDomainSocket()) {
unix_open();
} else {
local_open();
@@ -425,7 +425,7 @@
}
void TSocket::unix_open() {
- if (!path_.empty()) {
+ if (isUnixDomainSocket()) {
// Unix Domain Socket does not need addrinfo struct, so we pass NULL
openConnection(nullptr);
}
@@ -571,6 +571,7 @@
throw TTransportException(TTransportException::INTERRUPTED, "Interrupted");
}
} else /* ret == 0 */ {
+ GlobalOutput.perror("TSocket::read() THRIFT_EAGAIN (timed out) after %f ms", recvTimeout_);
throw TTransportException(TTransportException::TIMED_OUT, "THRIFT_EAGAIN (timed out)");
}
@@ -691,18 +692,22 @@
return b;
}
-std::string TSocket::getHost() {
+std::string TSocket::getHost() const {
return host_;
}
-int TSocket::getPort() {
+int TSocket::getPort() const {
return port_;
}
-std::string TSocket::getPath() {
+std::string TSocket::getPath() const {
return path_;
}
+bool TSocket::isUnixDomainSocket() const {
+ return !path_.empty();
+}
+
void TSocket::setHost(string host) {
host_ = host;
}
@@ -738,7 +743,7 @@
void TSocket::setNoDelay(bool noDelay) {
noDelay_ = noDelay;
- if (socket_ == THRIFT_INVALID_SOCKET || !path_.empty()) {
+ if (socket_ == THRIFT_INVALID_SOCKET || isUnixDomainSocket()) {
return;
}
@@ -816,7 +821,7 @@
string TSocket::getSocketInfo() const {
std::ostringstream oss;
- if (path_.empty()) {
+ if (!isUnixDomainSocket()) {
if (host_.empty() || port_ == 0) {
oss << "<Host: " << getPeerAddress();
oss << " Port: " << getPeerPort() << ">";
@@ -834,7 +839,7 @@
}
std::string TSocket::getPeerHost() const {
- if (peerHost_.empty() && path_.empty()) {
+ if (peerHost_.empty() && !isUnixDomainSocket()) {
struct sockaddr_storage addr;
struct sockaddr* addrPtr;
socklen_t addrLen;
@@ -872,7 +877,7 @@
}
std::string TSocket::getPeerAddress() const {
- if (peerAddress_.empty() && path_.empty()) {
+ if (peerAddress_.empty() && !isUnixDomainSocket()) {
struct sockaddr_storage addr;
struct sockaddr* addrPtr;
socklen_t addrLen;
@@ -916,7 +921,7 @@
}
void TSocket::setCachedAddress(const sockaddr* addr, socklen_t len) {
- if (!path_.empty()) {
+ if (isUnixDomainSocket()) {
return;
}
diff --git a/lib/cpp/src/thrift/transport/TSocket.h b/lib/cpp/src/thrift/transport/TSocket.h
index 8a224f2..f14546d 100644
--- a/lib/cpp/src/thrift/transport/TSocket.h
+++ b/lib/cpp/src/thrift/transport/TSocket.h
@@ -141,21 +141,29 @@
*
* @return string host identifier
*/
- std::string getHost();
+ std::string getHost() const;
/**
* Get the port that the socket is connected to
*
* @return int port number
*/
- int getPort();
+ int getPort() const;
/**
* Get the Unix domain socket path that the socket is connected to
*
* @return std::string path
*/
- std::string getPath();
+ std::string getPath() const;
+
+ /**
+ * Whether the socket is a Unix domain socket. This is the same as checking
+ * if getPath() is not empty.
+ *
+ * @return Is the socket a Unix domain socket?
+ */
+ bool isUnixDomainSocket() const;
/**
* Set the host that socket will connect to
@@ -285,7 +293,7 @@
* Constructor to create socket from file descriptor that
* can be interrupted safely.
*/
- TSocket(THRIFT_SOCKET socket, std::shared_ptr<THRIFT_SOCKET> interruptListener,
+ TSocket(THRIFT_SOCKET socket, std::shared_ptr<THRIFT_SOCKET> interruptListener,
std::shared_ptr<TConfiguration> config = nullptr);
/**
diff --git a/lib/cpp/test/CMakeLists.txt b/lib/cpp/test/CMakeLists.txt
index 07c178d..b57bb47 100644
--- a/lib/cpp/test/CMakeLists.txt
+++ b/lib/cpp/test/CMakeLists.txt
@@ -20,7 +20,7 @@
# Unit tests still require boost
include(BoostMacros)
REQUIRE_BOOST_HEADERS()
-set(BOOST_COMPONENTS filesystem thread unit_test_framework)
+set(BOOST_COMPONENTS filesystem thread unit_test_framework chrono)
REQUIRE_BOOST_LIBRARIES(BOOST_COMPONENTS)
include(ThriftMacros)
diff --git a/lib/cpp/test/SecurityFromBufferTest.cpp b/lib/cpp/test/SecurityFromBufferTest.cpp
index 72a4c2a..d275191 100644
--- a/lib/cpp/test/SecurityFromBufferTest.cpp
+++ b/lib/cpp/test/SecurityFromBufferTest.cpp
@@ -30,7 +30,7 @@
#include <thrift/transport/TSSLSocket.h>
#include <thrift/transport/TTransport.h>
#include <vector>
-#ifdef __linux__
+#ifdef HAVE_SIGNAL_H
#include <signal.h>
#endif
diff --git a/lib/cpp/test/SecurityTest.cpp b/lib/cpp/test/SecurityTest.cpp
index b70729c..cba8768 100644
--- a/lib/cpp/test/SecurityTest.cpp
+++ b/lib/cpp/test/SecurityTest.cpp
@@ -28,7 +28,7 @@
#include <thrift/transport/TSSLSocket.h>
#include <thrift/transport/TTransport.h>
#include <vector>
-#ifdef __linux__
+#ifdef HAVE_SIGNAL_H
#include <signal.h>
#endif
diff --git a/lib/cpp/test/TNonblockingSSLServerTest.cpp b/lib/cpp/test/TNonblockingSSLServerTest.cpp
index dc40c12..94289d5 100644
--- a/lib/cpp/test/TNonblockingSSLServerTest.cpp
+++ b/lib/cpp/test/TNonblockingSSLServerTest.cpp
@@ -29,6 +29,9 @@
#include "gen-cpp/ParentService.h"
#include <event.h>
+#ifdef HAVE_SIGNAL_H
+#include <signal.h>
+#endif
using namespace apache::thrift;
using apache::thrift::concurrency::Guard;
@@ -158,7 +161,7 @@
void run() override {
// When binding to explicit port, allow retrying to workaround bind failures on ports in use
int retryCount = port ? 10 : 0;
- pServerSocketFactory = createServerSocketFactory();
+ pServerSocketFactory = createServerSocketFactory();
startServer(retryCount);
}
diff --git a/lib/cpp/test/TSSLSocketInterruptTest.cpp b/lib/cpp/test/TSSLSocketInterruptTest.cpp
index 83fb993..bdaa466 100644
--- a/lib/cpp/test/TSSLSocketInterruptTest.cpp
+++ b/lib/cpp/test/TSSLSocketInterruptTest.cpp
@@ -27,7 +27,7 @@
#include <memory>
#include <thrift/transport/TSSLSocket.h>
#include <thrift/transport/TSSLServerSocket.h>
-#ifdef __linux__
+#ifdef HAVE_SIGNAL_H
#include <signal.h>
#endif