Repository: thrift Updated Branches: refs/heads/master 89cffc6f7 -> 24ea0bf5d
THRIFT-3130 - C++ Lib: removed no longer needed macro THRIFT_OVERLOAD_IF Client: C++ Patch: Jim King <[email protected]> This closes #483 Project: http://git-wip-us.apache.org/repos/asf/thrift/repo Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/24ea0bf5 Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/24ea0bf5 Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/24ea0bf5 Branch: refs/heads/master Commit: 24ea0bf5df0e431416fca897077af220a27b0320 Parents: 89cffc6 Author: Konrad Grochowski <[email protected]> Authored: Thu May 7 14:59:29 2015 +0200 Committer: Konrad Grochowski <[email protected]> Committed: Thu May 7 15:16:24 2015 +0200 ---------------------------------------------------------------------- lib/cpp/CMakeLists.txt | 2 +- lib/cpp/Makefile.am | 5 +- lib/cpp/src/thrift/TOutput.cpp | 126 +++++++++++++++++++ lib/cpp/src/thrift/TOutput.h | 59 +++++++++ lib/cpp/src/thrift/Thrift.cpp | 126 ------------------- lib/cpp/src/thrift/Thrift.h | 53 +------- lib/cpp/src/thrift/server/TNonblockingServer.h | 36 ++---- lib/cpp/src/thrift/server/TServer.h | 43 +++---- lib/cpp/src/thrift/server/TSimpleServer.cpp | 4 +- lib/cpp/src/thrift/server/TThreadPoolServer.cpp | 2 +- lib/cpp/src/thrift/server/TThreadedServer.cpp | 3 +- lib/cpp/test/TServerIntegrationTest.cpp | 107 +++++++++++++--- 12 files changed, 311 insertions(+), 255 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/thrift/blob/24ea0bf5/lib/cpp/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/lib/cpp/CMakeLists.txt b/lib/cpp/CMakeLists.txt index 46a48f7..84509a3 100755 --- a/lib/cpp/CMakeLists.txt +++ b/lib/cpp/CMakeLists.txt @@ -32,8 +32,8 @@ set(SYSLIBS "") # Create the thrift C++ library set( thriftcpp_SOURCES - src/thrift/Thrift.cpp src/thrift/TApplicationException.cpp + src/thrift/TOutput.cpp src/thrift/VirtualProfiling.cpp src/thrift/async/TAsyncChannel.cpp src/thrift/concurrency/ThreadManager.cpp http://git-wip-us.apache.org/repos/asf/thrift/blob/24ea0bf5/lib/cpp/Makefile.am ---------------------------------------------------------------------- diff --git a/lib/cpp/Makefile.am b/lib/cpp/Makefile.am index 0de8dc7..834ece2 100755 --- a/lib/cpp/Makefile.am +++ b/lib/cpp/Makefile.am @@ -62,8 +62,8 @@ AM_LDFLAGS = $(BOOST_LDFLAGS) $(OPENSSL_LDFLAGS) # Define the source files for the module -libthrift_la_SOURCES = src/thrift/Thrift.cpp \ - src/thrift/TApplicationException.cpp \ +libthrift_la_SOURCES = src/thrift/TApplicationException.cpp \ + src/thrift/TOutput.cpp \ src/thrift/VirtualProfiling.cpp \ src/thrift/async/TAsyncChannel.cpp \ src/thrift/concurrency/ThreadManager.cpp \ @@ -150,6 +150,7 @@ include_thrift_HEADERS = \ src/thrift/thrift-config.h \ src/thrift/TDispatchProcessor.h \ src/thrift/Thrift.h \ + src/thrift/TOutput.h \ src/thrift/TReflectionLocal.h \ src/thrift/TProcessor.h \ src/thrift/TApplicationException.h \ http://git-wip-us.apache.org/repos/asf/thrift/blob/24ea0bf5/lib/cpp/src/thrift/TOutput.cpp ---------------------------------------------------------------------- diff --git a/lib/cpp/src/thrift/TOutput.cpp b/lib/cpp/src/thrift/TOutput.cpp new file mode 100644 index 0000000..5739d0f --- /dev/null +++ b/lib/cpp/src/thrift/TOutput.cpp @@ -0,0 +1,126 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include <thrift/Thrift.h> +#include <cstring> +#include <cstdlib> +#include <boost/lexical_cast.hpp> +#include <stdarg.h> +#include <stdio.h> + +namespace apache { +namespace thrift { + +TOutput GlobalOutput; + +void TOutput::printf(const char* message, ...) { +#ifndef THRIFT_SQUELCH_CONSOLE_OUTPUT + // Try to reduce heap usage, even if printf is called rarely. + static const int STACK_BUF_SIZE = 256; + char stack_buf[STACK_BUF_SIZE]; + va_list ap; + +#ifdef _MSC_VER + va_start(ap, message); + int need = _vscprintf(message, ap); + va_end(ap); + + if (need < STACK_BUF_SIZE) { + va_start(ap, message); + vsnprintf_s(stack_buf, STACK_BUF_SIZE, _TRUNCATE, message, ap); + va_end(ap); + f_(stack_buf); + return; + } +#else + va_start(ap, message); + int need = vsnprintf(stack_buf, STACK_BUF_SIZE, message, ap); + va_end(ap); + + if (need < STACK_BUF_SIZE) { + f_(stack_buf); + return; + } +#endif + + char* heap_buf = (char*)malloc((need + 1) * sizeof(char)); + if (heap_buf == NULL) { +#ifdef _MSC_VER + va_start(ap, message); + vsnprintf_s(stack_buf, STACK_BUF_SIZE, _TRUNCATE, message, ap); + va_end(ap); +#endif + // Malloc failed. We might as well print the stack buffer. + f_(stack_buf); + return; + } + + va_start(ap, message); + int rval = vsnprintf(heap_buf, need + 1, message, ap); + va_end(ap); + // TODO(shigin): inform user + if (rval != -1) { + f_(heap_buf); + } + free(heap_buf); +#endif +} + +void TOutput::errorTimeWrapper(const char* msg) { +#ifndef THRIFT_SQUELCH_CONSOLE_OUTPUT + time_t now; + char dbgtime[26]; + time(&now); + THRIFT_CTIME_R(&now, dbgtime); + dbgtime[24] = 0; + fprintf(stderr, "Thrift: %s %s\n", dbgtime, msg); +#endif +} + +void TOutput::perror(const char* message, int errno_copy) { + std::string out = message + strerror_s(errno_copy); + f_(out.c_str()); +} + +std::string TOutput::strerror_s(int errno_copy) { +#ifndef HAVE_STRERROR_R + return "errno = " + boost::lexical_cast<std::string>(errno_copy); +#else // HAVE_STRERROR_R + + char b_errbuf[1024] = {'\0'}; +#ifdef STRERROR_R_CHAR_P + char* b_error = strerror_r(errno_copy, b_errbuf, sizeof(b_errbuf)); +#else + char* b_error = b_errbuf; + int rv = strerror_r(errno_copy, b_errbuf, sizeof(b_errbuf)); + if (rv == -1) { + // strerror_r failed. omgwtfbbq. + return "XSI-compliant strerror_r() failed with errno = " + + boost::lexical_cast<std::string>(errno_copy); + } +#endif + // Can anyone prove that explicit cast is probably not necessary + // to ensure that the string object is constructed before + // b_error becomes invalid? + return std::string(b_error); + +#endif // HAVE_STRERROR_R +} +} +} // apache::thrift http://git-wip-us.apache.org/repos/asf/thrift/blob/24ea0bf5/lib/cpp/src/thrift/TOutput.h ---------------------------------------------------------------------- diff --git a/lib/cpp/src/thrift/TOutput.h b/lib/cpp/src/thrift/TOutput.h new file mode 100644 index 0000000..9053b80 --- /dev/null +++ b/lib/cpp/src/thrift/TOutput.h @@ -0,0 +1,59 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#ifndef _THRIFT_OUTPUT_H_ +#define _THRIFT_OUTPUT_H_ 1 + +namespace apache { +namespace thrift { + +class TOutput { +public: + TOutput() : f_(&errorTimeWrapper) {} + + inline void setOutputFunction(void (*function)(const char*)) { f_ = function; } + + inline void operator()(const char* message) { f_(message); } + + // It is important to have a const char* overload here instead of + // just the string version, otherwise errno could be corrupted + // if there is some problem allocating memory when constructing + // the string. + void perror(const char* message, int errno_copy); + inline void perror(const std::string& message, int errno_copy) { + perror(message.c_str(), errno_copy); + } + + void printf(const char* message, ...); + + static void errorTimeWrapper(const char* msg); + + /** Just like strerror_r but returns a C++ string object. */ + static std::string strerror_s(int errno_copy); + +private: + void (*f_)(const char*); +}; + +extern TOutput GlobalOutput; + +} +} // namespace apache::thrift + +#endif //_THRIFT_OUTPUT_H_ http://git-wip-us.apache.org/repos/asf/thrift/blob/24ea0bf5/lib/cpp/src/thrift/Thrift.cpp ---------------------------------------------------------------------- diff --git a/lib/cpp/src/thrift/Thrift.cpp b/lib/cpp/src/thrift/Thrift.cpp deleted file mode 100644 index 5739d0f..0000000 --- a/lib/cpp/src/thrift/Thrift.cpp +++ /dev/null @@ -1,126 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -#include <thrift/Thrift.h> -#include <cstring> -#include <cstdlib> -#include <boost/lexical_cast.hpp> -#include <stdarg.h> -#include <stdio.h> - -namespace apache { -namespace thrift { - -TOutput GlobalOutput; - -void TOutput::printf(const char* message, ...) { -#ifndef THRIFT_SQUELCH_CONSOLE_OUTPUT - // Try to reduce heap usage, even if printf is called rarely. - static const int STACK_BUF_SIZE = 256; - char stack_buf[STACK_BUF_SIZE]; - va_list ap; - -#ifdef _MSC_VER - va_start(ap, message); - int need = _vscprintf(message, ap); - va_end(ap); - - if (need < STACK_BUF_SIZE) { - va_start(ap, message); - vsnprintf_s(stack_buf, STACK_BUF_SIZE, _TRUNCATE, message, ap); - va_end(ap); - f_(stack_buf); - return; - } -#else - va_start(ap, message); - int need = vsnprintf(stack_buf, STACK_BUF_SIZE, message, ap); - va_end(ap); - - if (need < STACK_BUF_SIZE) { - f_(stack_buf); - return; - } -#endif - - char* heap_buf = (char*)malloc((need + 1) * sizeof(char)); - if (heap_buf == NULL) { -#ifdef _MSC_VER - va_start(ap, message); - vsnprintf_s(stack_buf, STACK_BUF_SIZE, _TRUNCATE, message, ap); - va_end(ap); -#endif - // Malloc failed. We might as well print the stack buffer. - f_(stack_buf); - return; - } - - va_start(ap, message); - int rval = vsnprintf(heap_buf, need + 1, message, ap); - va_end(ap); - // TODO(shigin): inform user - if (rval != -1) { - f_(heap_buf); - } - free(heap_buf); -#endif -} - -void TOutput::errorTimeWrapper(const char* msg) { -#ifndef THRIFT_SQUELCH_CONSOLE_OUTPUT - time_t now; - char dbgtime[26]; - time(&now); - THRIFT_CTIME_R(&now, dbgtime); - dbgtime[24] = 0; - fprintf(stderr, "Thrift: %s %s\n", dbgtime, msg); -#endif -} - -void TOutput::perror(const char* message, int errno_copy) { - std::string out = message + strerror_s(errno_copy); - f_(out.c_str()); -} - -std::string TOutput::strerror_s(int errno_copy) { -#ifndef HAVE_STRERROR_R - return "errno = " + boost::lexical_cast<std::string>(errno_copy); -#else // HAVE_STRERROR_R - - char b_errbuf[1024] = {'\0'}; -#ifdef STRERROR_R_CHAR_P - char* b_error = strerror_r(errno_copy, b_errbuf, sizeof(b_errbuf)); -#else - char* b_error = b_errbuf; - int rv = strerror_r(errno_copy, b_errbuf, sizeof(b_errbuf)); - if (rv == -1) { - // strerror_r failed. omgwtfbbq. - return "XSI-compliant strerror_r() failed with errno = " - + boost::lexical_cast<std::string>(errno_copy); - } -#endif - // Can anyone prove that explicit cast is probably not necessary - // to ensure that the string object is constructed before - // b_error becomes invalid? - return std::string(b_error); - -#endif // HAVE_STRERROR_R -} -} -} // apache::thrift http://git-wip-us.apache.org/repos/asf/thrift/blob/24ea0bf5/lib/cpp/src/thrift/Thrift.h ---------------------------------------------------------------------- diff --git a/lib/cpp/src/thrift/Thrift.h b/lib/cpp/src/thrift/Thrift.h index 9ddf946..962b921 100644 --- a/lib/cpp/src/thrift/Thrift.h +++ b/lib/cpp/src/thrift/Thrift.h @@ -46,26 +46,7 @@ #include <boost/type_traits/is_convertible.hpp> #include <thrift/TLogging.h> - -/** - * Helper macros to allow function overloading even when using - * boost::shared_ptr. - * - * shared_ptr makes overloading really annoying, since shared_ptr defines - * constructor methods to allow one shared_ptr type to be constructed from any - * other shared_ptr type. (Even if it would be a compile error to actually try - * to instantiate the constructor.) These macros add an extra argument to the - * function to cause it to only be instantiated if a pointer of type T is - * convertible to a pointer of type U. - * - * THRIFT_OVERLOAD_IF should be used in function declarations. - * THRIFT_OVERLOAD_IF_DEFN should be used in the function definition, if it is - * defined separately from where it is declared. - */ -#define THRIFT_OVERLOAD_IF_DEFN(T, Y) \ - typename ::boost::enable_if<typename ::boost::is_convertible<T*, Y*>::type, void*>::type - -#define THRIFT_OVERLOAD_IF(T, Y) THRIFT_OVERLOAD_IF_DEFN(T, Y) = NULL +#include <thrift/TOutput.h> #define THRIFT_UNUSED_VARIABLE(x) ((void)(x)) @@ -81,7 +62,7 @@ public: int operator++() { return ++ii_; } bool operator!=(const TEnumIterator& end) { - (void)end; // avoid "unused" warning with NDEBUG + THRIFT_UNUSED_VARIABLE(end); assert(end.n_ == -1); return (ii_ != n_); } @@ -95,36 +76,6 @@ private: const char** names_; }; -class TOutput { -public: - TOutput() : f_(&errorTimeWrapper) {} - - inline void setOutputFunction(void (*function)(const char*)) { f_ = function; } - - inline void operator()(const char* message) { f_(message); } - - // It is important to have a const char* overload here instead of - // just the string version, otherwise errno could be corrupted - // if there is some problem allocating memory when constructing - // the string. - void perror(const char* message, int errno_copy); - inline void perror(const std::string& message, int errno_copy) { - perror(message.c_str(), errno_copy); - } - - void printf(const char* message, ...); - - static void errorTimeWrapper(const char* msg); - - /** Just like strerror_r but returns a C++ string object. */ - static std::string strerror_s(int errno_copy); - -private: - void (*f_)(const char*); -}; - -extern TOutput GlobalOutput; - class TException : public std::exception { public: TException() : message_() {} http://git-wip-us.apache.org/repos/asf/thrift/blob/24ea0bf5/lib/cpp/src/thrift/server/TNonblockingServer.h ---------------------------------------------------------------------- diff --git a/lib/cpp/src/thrift/server/TNonblockingServer.h b/lib/cpp/src/thrift/server/TNonblockingServer.h index 0a0d167..7922a68 100644 --- a/lib/cpp/src/thrift/server/TNonblockingServer.h +++ b/lib/cpp/src/thrift/server/TNonblockingServer.h @@ -305,29 +305,23 @@ private: } public: - template <typename ProcessorFactory> - TNonblockingServer(const boost::shared_ptr<ProcessorFactory>& processorFactory, - int port, - THRIFT_OVERLOAD_IF(ProcessorFactory, TProcessorFactory)) + TNonblockingServer(const boost::shared_ptr<TProcessorFactory>& processorFactory, + int port) : TServer(processorFactory) { init(port); } - template <typename Processor> - TNonblockingServer(const boost::shared_ptr<Processor>& processor, - int port, - THRIFT_OVERLOAD_IF(Processor, TProcessor)) + TNonblockingServer(const boost::shared_ptr<TProcessor>& processor, + int port) : TServer(processor) { init(port); } - template <typename ProcessorFactory> - TNonblockingServer(const boost::shared_ptr<ProcessorFactory>& processorFactory, + TNonblockingServer(const boost::shared_ptr<TProcessorFactory>& processorFactory, const boost::shared_ptr<TProtocolFactory>& protocolFactory, int port, const boost::shared_ptr<ThreadManager>& threadManager - = boost::shared_ptr<ThreadManager>(), - THRIFT_OVERLOAD_IF(ProcessorFactory, TProcessorFactory)) + = boost::shared_ptr<ThreadManager>()) : TServer(processorFactory) { init(port); @@ -337,13 +331,11 @@ public: setThreadManager(threadManager); } - template <typename Processor> - TNonblockingServer(const boost::shared_ptr<Processor>& processor, + TNonblockingServer(const boost::shared_ptr<TProcessor>& processor, const boost::shared_ptr<TProtocolFactory>& protocolFactory, int port, const boost::shared_ptr<ThreadManager>& threadManager - = boost::shared_ptr<ThreadManager>(), - THRIFT_OVERLOAD_IF(Processor, TProcessor)) + = boost::shared_ptr<ThreadManager>()) : TServer(processor) { init(port); @@ -353,16 +345,14 @@ public: setThreadManager(threadManager); } - template <typename ProcessorFactory> - TNonblockingServer(const boost::shared_ptr<ProcessorFactory>& processorFactory, + TNonblockingServer(const boost::shared_ptr<TProcessorFactory>& processorFactory, const boost::shared_ptr<TTransportFactory>& inputTransportFactory, const boost::shared_ptr<TTransportFactory>& outputTransportFactory, const boost::shared_ptr<TProtocolFactory>& inputProtocolFactory, const boost::shared_ptr<TProtocolFactory>& outputProtocolFactory, int port, const boost::shared_ptr<ThreadManager>& threadManager - = boost::shared_ptr<ThreadManager>(), - THRIFT_OVERLOAD_IF(ProcessorFactory, TProcessorFactory)) + = boost::shared_ptr<ThreadManager>()) : TServer(processorFactory) { init(port); @@ -374,16 +364,14 @@ public: setThreadManager(threadManager); } - template <typename Processor> - TNonblockingServer(const boost::shared_ptr<Processor>& processor, + TNonblockingServer(const boost::shared_ptr<TProcessor>& processor, const boost::shared_ptr<TTransportFactory>& inputTransportFactory, const boost::shared_ptr<TTransportFactory>& outputTransportFactory, const boost::shared_ptr<TProtocolFactory>& inputProtocolFactory, const boost::shared_ptr<TProtocolFactory>& outputProtocolFactory, int port, const boost::shared_ptr<ThreadManager>& threadManager - = boost::shared_ptr<ThreadManager>(), - THRIFT_OVERLOAD_IF(Processor, TProcessor)) + = boost::shared_ptr<ThreadManager>()) : TServer(processor) { init(port); http://git-wip-us.apache.org/repos/asf/thrift/blob/24ea0bf5/lib/cpp/src/thrift/server/TServer.h ---------------------------------------------------------------------- diff --git a/lib/cpp/src/thrift/server/TServer.h b/lib/cpp/src/thrift/server/TServer.h index c0b222f..47e0d40 100644 --- a/lib/cpp/src/thrift/server/TServer.h +++ b/lib/cpp/src/thrift/server/TServer.h @@ -124,9 +124,7 @@ public: boost::shared_ptr<TServerEventHandler> getEventHandler() { return eventHandler_; } protected: - template <typename ProcessorFactory> - TServer(const boost::shared_ptr<ProcessorFactory>& processorFactory, - THRIFT_OVERLOAD_IF(ProcessorFactory, TProcessorFactory)) + TServer(const boost::shared_ptr<TProcessorFactory>& processorFactory) : processorFactory_(processorFactory) { setInputTransportFactory(boost::shared_ptr<TTransportFactory>(new TTransportFactory())); setOutputTransportFactory(boost::shared_ptr<TTransportFactory>(new TTransportFactory())); @@ -134,8 +132,7 @@ protected: setOutputProtocolFactory(boost::shared_ptr<TProtocolFactory>(new TBinaryProtocolFactory())); } - template <typename Processor> - TServer(const boost::shared_ptr<Processor>& processor, THRIFT_OVERLOAD_IF(Processor, TProcessor)) + TServer(const boost::shared_ptr<TProcessor>& processor) : processorFactory_(new TSingletonProcessorFactory(processor)) { setInputTransportFactory(boost::shared_ptr<TTransportFactory>(new TTransportFactory())); setOutputTransportFactory(boost::shared_ptr<TTransportFactory>(new TTransportFactory())); @@ -143,10 +140,8 @@ protected: setOutputProtocolFactory(boost::shared_ptr<TProtocolFactory>(new TBinaryProtocolFactory())); } - template <typename ProcessorFactory> - TServer(const boost::shared_ptr<ProcessorFactory>& processorFactory, - const boost::shared_ptr<TServerTransport>& serverTransport, - THRIFT_OVERLOAD_IF(ProcessorFactory, TProcessorFactory)) + TServer(const boost::shared_ptr<TProcessorFactory>& processorFactory, + const boost::shared_ptr<TServerTransport>& serverTransport) : processorFactory_(processorFactory), serverTransport_(serverTransport) { setInputTransportFactory(boost::shared_ptr<TTransportFactory>(new TTransportFactory())); setOutputTransportFactory(boost::shared_ptr<TTransportFactory>(new TTransportFactory())); @@ -154,10 +149,8 @@ protected: setOutputProtocolFactory(boost::shared_ptr<TProtocolFactory>(new TBinaryProtocolFactory())); } - template <typename Processor> - TServer(const boost::shared_ptr<Processor>& processor, - const boost::shared_ptr<TServerTransport>& serverTransport, - THRIFT_OVERLOAD_IF(Processor, TProcessor)) + TServer(const boost::shared_ptr<TProcessor>& processor, + const boost::shared_ptr<TServerTransport>& serverTransport) : processorFactory_(new TSingletonProcessorFactory(processor)), serverTransport_(serverTransport) { setInputTransportFactory(boost::shared_ptr<TTransportFactory>(new TTransportFactory())); @@ -166,12 +159,10 @@ protected: setOutputProtocolFactory(boost::shared_ptr<TProtocolFactory>(new TBinaryProtocolFactory())); } - template <typename ProcessorFactory> - TServer(const boost::shared_ptr<ProcessorFactory>& processorFactory, + TServer(const boost::shared_ptr<TProcessorFactory>& processorFactory, const boost::shared_ptr<TServerTransport>& serverTransport, const boost::shared_ptr<TTransportFactory>& transportFactory, - const boost::shared_ptr<TProtocolFactory>& protocolFactory, - THRIFT_OVERLOAD_IF(ProcessorFactory, TProcessorFactory)) + const boost::shared_ptr<TProtocolFactory>& protocolFactory) : processorFactory_(processorFactory), serverTransport_(serverTransport), inputTransportFactory_(transportFactory), @@ -179,12 +170,10 @@ protected: inputProtocolFactory_(protocolFactory), outputProtocolFactory_(protocolFactory) {} - template <typename Processor> - TServer(const boost::shared_ptr<Processor>& processor, + TServer(const boost::shared_ptr<TProcessor>& processor, const boost::shared_ptr<TServerTransport>& serverTransport, const boost::shared_ptr<TTransportFactory>& transportFactory, - const boost::shared_ptr<TProtocolFactory>& protocolFactory, - THRIFT_OVERLOAD_IF(Processor, TProcessor)) + const boost::shared_ptr<TProtocolFactory>& protocolFactory) : processorFactory_(new TSingletonProcessorFactory(processor)), serverTransport_(serverTransport), inputTransportFactory_(transportFactory), @@ -192,14 +181,12 @@ protected: inputProtocolFactory_(protocolFactory), outputProtocolFactory_(protocolFactory) {} - template <typename ProcessorFactory> - TServer(const boost::shared_ptr<ProcessorFactory>& processorFactory, + TServer(const boost::shared_ptr<TProcessorFactory>& processorFactory, const boost::shared_ptr<TServerTransport>& serverTransport, const boost::shared_ptr<TTransportFactory>& inputTransportFactory, const boost::shared_ptr<TTransportFactory>& outputTransportFactory, const boost::shared_ptr<TProtocolFactory>& inputProtocolFactory, - const boost::shared_ptr<TProtocolFactory>& outputProtocolFactory, - THRIFT_OVERLOAD_IF(ProcessorFactory, TProcessorFactory)) + const boost::shared_ptr<TProtocolFactory>& outputProtocolFactory) : processorFactory_(processorFactory), serverTransport_(serverTransport), inputTransportFactory_(inputTransportFactory), @@ -207,14 +194,12 @@ protected: inputProtocolFactory_(inputProtocolFactory), outputProtocolFactory_(outputProtocolFactory) {} - template <typename Processor> - TServer(const boost::shared_ptr<Processor>& processor, + TServer(const boost::shared_ptr<TProcessor>& processor, const boost::shared_ptr<TServerTransport>& serverTransport, const boost::shared_ptr<TTransportFactory>& inputTransportFactory, const boost::shared_ptr<TTransportFactory>& outputTransportFactory, const boost::shared_ptr<TProtocolFactory>& inputProtocolFactory, - const boost::shared_ptr<TProtocolFactory>& outputProtocolFactory, - THRIFT_OVERLOAD_IF(Processor, TProcessor)) + const boost::shared_ptr<TProtocolFactory>& outputProtocolFactory) : processorFactory_(new TSingletonProcessorFactory(processor)), serverTransport_(serverTransport), inputTransportFactory_(inputTransportFactory), http://git-wip-us.apache.org/repos/asf/thrift/blob/24ea0bf5/lib/cpp/src/thrift/server/TSimpleServer.cpp ---------------------------------------------------------------------- diff --git a/lib/cpp/src/thrift/server/TSimpleServer.cpp b/lib/cpp/src/thrift/server/TSimpleServer.cpp index adcedc8..3b04f35 100644 --- a/lib/cpp/src/thrift/server/TSimpleServer.cpp +++ b/lib/cpp/src/thrift/server/TSimpleServer.cpp @@ -92,13 +92,13 @@ void TSimpleServer::onClientConnected(const shared_ptr<TConnectedClient>& pClien /** * TSimpleServer does not track clients so there is nothing to do here. */ -void TSimpleServer::onClientDisconnected(TConnectedClient *pClient) {} +void TSimpleServer::onClientDisconnected(TConnectedClient*) {} /** * This makes little sense to the simple server because it is not capable * of having more than one client at a time, so we hide it. */ -void TSimpleServer::setConcurrentClientLimit(int64_t newLimit) {} +void TSimpleServer::setConcurrentClientLimit(int64_t) {} } http://git-wip-us.apache.org/repos/asf/thrift/blob/24ea0bf5/lib/cpp/src/thrift/server/TThreadPoolServer.cpp ---------------------------------------------------------------------- diff --git a/lib/cpp/src/thrift/server/TThreadPoolServer.cpp b/lib/cpp/src/thrift/server/TThreadPoolServer.cpp index 5b9b01d..60634ef 100644 --- a/lib/cpp/src/thrift/server/TThreadPoolServer.cpp +++ b/lib/cpp/src/thrift/server/TThreadPoolServer.cpp @@ -120,7 +120,7 @@ void TThreadPoolServer::onClientConnected(const shared_ptr<TConnectedClient>& pC threadManager_->add(pClient, timeout_, taskExpiration_); } -void TThreadPoolServer::onClientDisconnected(TConnectedClient *pClient) {} +void TThreadPoolServer::onClientDisconnected(TConnectedClient*) {} } } http://git-wip-us.apache.org/repos/asf/thrift/blob/24ea0bf5/lib/cpp/src/thrift/server/TThreadedServer.cpp ---------------------------------------------------------------------- diff --git a/lib/cpp/src/thrift/server/TThreadedServer.cpp b/lib/cpp/src/thrift/server/TThreadedServer.cpp index b0b22c3..98b02c5 100644 --- a/lib/cpp/src/thrift/server/TThreadedServer.cpp +++ b/lib/cpp/src/thrift/server/TThreadedServer.cpp @@ -102,7 +102,8 @@ void TThreadedServer::onClientConnected(const shared_ptr<TConnectedClient>& pCli threadFactory_->newThread(pClient)->start(); } -void TThreadedServer::onClientDisconnected(TConnectedClient *pClient) { +void TThreadedServer::onClientDisconnected(TConnectedClient* pClient) { + THRIFT_UNUSED_VARIABLE(pClient); Synchronized s(clientsMonitor_); if (getConcurrentClientCount() == 0) { clientsMonitor_.notify(); http://git-wip-us.apache.org/repos/asf/thrift/blob/24ea0bf5/lib/cpp/test/TServerIntegrationTest.cpp ---------------------------------------------------------------------- diff --git a/lib/cpp/test/TServerIntegrationTest.cpp b/lib/cpp/test/TServerIntegrationTest.cpp index 73bcdba..fec1b9c 100644 --- a/lib/cpp/test/TServerIntegrationTest.cpp +++ b/lib/cpp/test/TServerIntegrationTest.cpp @@ -22,6 +22,7 @@ #include <boost/bind.hpp> #include <boost/foreach.hpp> #include <boost/format.hpp> +#include <boost/make_shared.hpp> #include <boost/shared_ptr.hpp> #include <boost/thread.hpp> #include <thrift/server/TSimpleServer.h> @@ -56,7 +57,12 @@ using apache::thrift::server::TThreadPoolServer; using apache::thrift::server::TThreadedServer; using apache::thrift::test::ParentServiceClient; using apache::thrift::test::ParentServiceIf; +using apache::thrift::test::ParentServiceIfFactory; +using apache::thrift::test::ParentServiceIfSingletonFactory; using apache::thrift::test::ParentServiceProcessor; +using apache::thrift::test::ParentServiceProcessorFactory; +using apache::thrift::TProcessor; +using apache::thrift::TProcessorFactory; using boost::posix_time::milliseconds; /** @@ -117,15 +123,19 @@ public: } void getDataWait(std::string& _return, int32_t length) { + THRIFT_UNUSED_VARIABLE(_return); + THRIFT_UNUSED_VARIABLE(length); } void onewayWait() { } void exceptionWait(const std::string& message) { + THRIFT_UNUSED_VARIABLE(message); } void unexpectedExceptionWait(const std::string& message) { + THRIFT_UNUSED_VARIABLE(message); } protected: @@ -143,10 +153,9 @@ template<class TServerType> class TServerIntegrationTestFixture : public TestPortFixture { public: - TServerIntegrationTestFixture() : + TServerIntegrationTestFixture(const boost::shared_ptr<TProcessorFactory>& _processorFactory) : pServer(new TServerType( - boost::shared_ptr<ParentServiceProcessor>(new ParentServiceProcessor( - boost::shared_ptr<ParentServiceIf>(new ParentHandler))), + _processorFactory, boost::shared_ptr<TServerTransport>(new TServerSocket("localhost", m_serverPort)), boost::shared_ptr<TTransportFactory>(new TTransportFactory), boost::shared_ptr<TProtocolFactory>(new TBinaryProtocolFactory))), @@ -155,6 +164,17 @@ public: pServer->setServerEventHandler(pEventHandler); } + TServerIntegrationTestFixture(const boost::shared_ptr<TProcessor>& _processor) : + pServer(new TServerType( + _processor, + boost::shared_ptr<TServerTransport>(new TServerSocket("localhost", 0)), + boost::shared_ptr<TTransportFactory>(new TTransportFactory), + boost::shared_ptr<TProtocolFactory>(new TBinaryProtocolFactory))), + pEventHandler(boost::shared_ptr<TServerReadyEventHandler>(new TServerReadyEventHandler)) + { + pServer->setServerEventHandler(pEventHandler); + } + void startServer() { pServerThread.reset(new boost::thread(boost::bind(&TServerType::serve, pServer.get()))); @@ -191,6 +211,11 @@ public: stopServer(); } + int getServerPort() { + TServerSocket *pSock = dynamic_cast<TServerSocket *>(pServer->getServerTransport().get()); + return pSock->getPort(); + } + void delayClose(boost::shared_ptr<TTransport> toClose, boost::posix_time::time_duration after) { boost::this_thread::sleep(after); toClose->close(); @@ -202,7 +227,7 @@ public: std::vector<boost::shared_ptr<boost::thread> > holdThreads; for (int64_t i = 0; i < numToMake; ++i) { - boost::shared_ptr<TSocket> pClientSock(new TSocket("localhost", m_serverPort), autoSocketCloser); + boost::shared_ptr<TSocket> pClientSock(new TSocket("localhost", getServerPort()), autoSocketCloser); holdSockets.push_back(pClientSock); boost::shared_ptr<TProtocol> pClientProtocol(new TBinaryProtocol(pClientSock)); ParentServiceClient client(pClientProtocol); @@ -229,25 +254,71 @@ public: boost::shared_ptr<boost::thread> pServerThread; }; -BOOST_FIXTURE_TEST_SUITE( Baseline, TestPortFixture ) +template<class TServerType> +class TServerIntegrationProcessorFactoryTestFixture : public TServerIntegrationTestFixture<TServerType> +{ +public: + TServerIntegrationProcessorFactoryTestFixture() : + TServerIntegrationTestFixture<TServerType>( + boost::make_shared<ParentServiceProcessorFactory>( + boost::make_shared<ParentServiceIfSingletonFactory>( + boost::make_shared<ParentHandler>()))) { } +}; + +template<class TServerType> +class TServerIntegrationProcessorTestFixture : public TServerIntegrationTestFixture<TServerType> +{ +public: + TServerIntegrationProcessorTestFixture() : + TServerIntegrationTestFixture<TServerType>( + boost::make_shared<ParentServiceProcessor>( + boost::make_shared<ParentHandler>())) { } +}; + +BOOST_AUTO_TEST_SUITE(constructors) + +BOOST_FIXTURE_TEST_CASE(test_simple_factory, TServerIntegrationProcessorFactoryTestFixture<TSimpleServer>) +{ + baseline(3, 1); +} -BOOST_FIXTURE_TEST_CASE(test_simple, TServerIntegrationTestFixture<TSimpleServer>) +BOOST_FIXTURE_TEST_CASE(test_simple, TServerIntegrationProcessorTestFixture<TSimpleServer>) { baseline(3, 1); } -BOOST_FIXTURE_TEST_CASE(test_threaded, TServerIntegrationTestFixture<TThreadedServer>) +BOOST_FIXTURE_TEST_CASE(test_threaded_factory, TServerIntegrationProcessorFactoryTestFixture<TThreadedServer>) { baseline(10, 10); } -BOOST_FIXTURE_TEST_CASE(test_threaded_bound, TServerIntegrationTestFixture<TThreadedServer>) +BOOST_FIXTURE_TEST_CASE(test_threaded, TServerIntegrationProcessorTestFixture<TThreadedServer>) +{ + baseline(10, 10); +} + +BOOST_FIXTURE_TEST_CASE(test_threaded_bound, TServerIntegrationProcessorTestFixture<TThreadedServer>) { pServer->setConcurrentClientLimit(4); baseline(10, 4); } -BOOST_FIXTURE_TEST_CASE(test_threadpool, TServerIntegrationTestFixture<TThreadPoolServer>) +BOOST_FIXTURE_TEST_CASE(test_threadpool_factory, TServerIntegrationProcessorFactoryTestFixture<TThreadPoolServer>) +{ + pServer->getThreadManager()->threadFactory( + boost::shared_ptr<apache::thrift::concurrency::ThreadFactory>( + new apache::thrift::concurrency::PlatformThreadFactory)); + pServer->getThreadManager()->start(); + + // thread factory has 4 threads as a default + // thread factory however is a bad way to limit concurrent clients + // as accept() will be called to grab a 5th client socket, in this case + // and then the thread factory will block adding the thread to manage + // that client. + baseline(10, 5); +} + +BOOST_FIXTURE_TEST_CASE(test_threadpool, TServerIntegrationProcessorTestFixture<TThreadPoolServer>) { pServer->getThreadManager()->threadFactory( boost::shared_ptr<apache::thrift::concurrency::ThreadFactory>( @@ -262,7 +333,7 @@ BOOST_FIXTURE_TEST_CASE(test_threadpool, TServerIntegrationTestFixture<TThreadPo baseline(10, 5); } -BOOST_FIXTURE_TEST_CASE(test_threadpool_bound, TServerIntegrationTestFixture<TThreadPoolServer>) +BOOST_FIXTURE_TEST_CASE(test_threadpool_bound, TServerIntegrationProcessorTestFixture<TThreadPoolServer>) { pServer->getThreadManager()->threadFactory( boost::shared_ptr<apache::thrift::concurrency::ThreadFactory>( @@ -276,7 +347,7 @@ BOOST_FIXTURE_TEST_CASE(test_threadpool_bound, TServerIntegrationTestFixture<TTh BOOST_AUTO_TEST_SUITE_END() -BOOST_FIXTURE_TEST_SUITE ( TServerIntegrationTest, TServerIntegrationTestFixture<TThreadedServer> ) +BOOST_FIXTURE_TEST_SUITE ( TServerIntegrationTest, TServerIntegrationProcessorTestFixture<TThreadedServer> ) BOOST_AUTO_TEST_CASE(test_stop_with_interruptable_clients_connected) { @@ -284,10 +355,10 @@ BOOST_AUTO_TEST_CASE(test_stop_with_interruptable_clients_connected) startServer(); - boost::shared_ptr<TSocket> pClientSock1(new TSocket("localhost", m_serverPort), autoSocketCloser); + boost::shared_ptr<TSocket> pClientSock1(new TSocket("localhost", getServerPort()), autoSocketCloser); pClientSock1->open(); - boost::shared_ptr<TSocket> pClientSock2(new TSocket("localhost", m_serverPort), autoSocketCloser); + boost::shared_ptr<TSocket> pClientSock2(new TSocket("localhost", getServerPort()), autoSocketCloser); pClientSock2->open(); // Ensure they have been accepted @@ -313,10 +384,10 @@ BOOST_AUTO_TEST_CASE(test_stop_with_uninterruptable_clients_connected) startServer(); - boost::shared_ptr<TSocket> pClientSock1(new TSocket("localhost", m_serverPort), autoSocketCloser); + boost::shared_ptr<TSocket> pClientSock1(new TSocket("localhost", getServerPort()), autoSocketCloser); pClientSock1->open(); - boost::shared_ptr<TSocket> pClientSock2(new TSocket("localhost", m_serverPort), autoSocketCloser); + boost::shared_ptr<TSocket> pClientSock2(new TSocket("localhost", getServerPort()), autoSocketCloser); pClientSock2->open(); // Ensure they have been accepted @@ -340,19 +411,19 @@ BOOST_AUTO_TEST_CASE(test_concurrent_client_limit) BOOST_CHECK_EQUAL(0, pServer->getConcurrentClientCount()); BOOST_CHECK_EQUAL(2, pServer->getConcurrentClientLimit()); - boost::shared_ptr<TSocket> pClientSock1(new TSocket("localhost", m_serverPort), autoSocketCloser); + boost::shared_ptr<TSocket> pClientSock1(new TSocket("localhost", getServerPort()), autoSocketCloser); pClientSock1->open(); blockUntilAccepted(1); BOOST_CHECK_EQUAL(1, pServer->getConcurrentClientCount()); - boost::shared_ptr<TSocket> pClientSock2(new TSocket("localhost", m_serverPort), autoSocketCloser); + boost::shared_ptr<TSocket> pClientSock2(new TSocket("localhost", getServerPort()), autoSocketCloser); pClientSock2->open(); blockUntilAccepted(2); BOOST_CHECK_EQUAL(2, pServer->getConcurrentClientCount()); // a third client cannot connect until one of the other two closes boost::thread t2(boost::bind(&TServerIntegrationTestFixture::delayClose, this, pClientSock2, milliseconds(250))); - boost::shared_ptr<TSocket> pClientSock3(new TSocket("localhost", m_serverPort), autoSocketCloser); + boost::shared_ptr<TSocket> pClientSock3(new TSocket("localhost", getServerPort()), autoSocketCloser); pClientSock2->open(); blockUntilAccepted(2); BOOST_CHECK_EQUAL(2, pServer->getConcurrentClientCount());
