[
https://issues.apache.org/jira/browse/THRIFT-3768?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15225570#comment-15225570
]
ASF GitHub Bot commented on THRIFT-3768:
----------------------------------------
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/977#discussion_r58481087
--- Diff: lib/cpp/src/thrift/server/TThreadedServer.cpp ---
@@ -77,44 +83,29 @@ TThreadedServer::TThreadedServer(const
shared_ptr<TProcessor>& processor,
const shared_ptr<TProtocolFactory>&
inputProtocolFactory,
const shared_ptr<TProtocolFactory>&
outputProtocolFactory,
const shared_ptr<ThreadFactory>&
threadFactory)
- : TServerFramework(processor,
- serverTransport,
- inputTransportFactory,
- outputTransportFactory,
- inputProtocolFactory,
- outputProtocolFactory),
- threadFactory_(threadFactory) {
+ : TThreadPoolServer(processor,
+ serverTransport,
+ inputTransportFactory,
+ outputTransportFactory,
+ inputProtocolFactory,
+ outputProtocolFactory,
+
apache::thrift::concurrency::ThreadManager::newSimpleThreadManager(0, 0)) {
+ threadManager_->threadFactory(threadFactory);
+ threadManager_->start();
}
TThreadedServer::~TThreadedServer() {
}
-void TThreadedServer::serve() {
- TServerFramework::serve();
-
- // Drain all clients - no more will arrive
- try {
- Synchronized s(clientsMonitor_);
- while (getConcurrentClientCount() > 0) {
- clientsMonitor_.wait();
- }
- } catch (TException& tx) {
- string errStr = string("TThreadedServer: Exception joining workers: ")
+ tx.what();
- GlobalOutput(errStr.c_str());
+void TThreadedServer::onClientConnected(const
shared_ptr<TConnectedClient>& pClient) {
+ if (!threadManager_->idleWorkerCount())
--- End diff --
As submitted the number of threads will grow to the maximum number of
concurrent connections and will not shrink. Additional work is needed to allow
ThreadManager::removeWorker() to do the right thing when many threads arrive at
it at the same time. In gdb I found that workerCount_ was 10 but
workerMaxCount_ was 0; the stop behavior for ThreadManager calls
removeWorker(workerCount_) and gets an exception, which eventually causes a
core. This is why, for this submission, I left it as stated.
> TThreadedServer may crash if it is destroyed immediately after it returns
> from serve()
> --------------------------------------------------------------------------------------
>
> Key: THRIFT-3768
> URL: https://issues.apache.org/jira/browse/THRIFT-3768
> Project: Thrift
> Issue Type: Bug
> Components: C++ - Library
> Affects Versions: 0.9.3
> Reporter: Ted Wang
> Assignee: James E. King, III
> Priority: Minor
>
> Here's a sequence that shows the race:
> Thread-1 (Users of TThreadedServer): Calls TThreadedServer::stop(), which
> calls interruptChildren and initiates the tearing down of client connections.
> Thread-2: In TServerFramework::serve(), broke out of accept, and now blocks
> in TThreadedServer::serve() waiting to drain all the clients.
> Thread-3 (The connected client thread created by TThreadedServer): In
> disposeConnectedClient, running because the server is shutting down and the
> shared_ptr specified this function to be the cleanup function for the client.
> This thread just returned from onClientDisconnected and now context switches.
> Thread-2: TThreadedServer::serve() is notified that all of the clients have
> disconnected and completes.
> Thread-1: Joins on Thread-2 and destroys the server object because it is done.
> Thread-3: Finally gets a chance to run, but now encounters undefined behavior
> because it is still executing a member function of an object that has been
> destroyed.
> You can force this race in action if you put sleep(1) before
> onClientDisconnected() in disposeConnectedClient
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
