[ https://issues.apache.org/jira/browse/THRIFT-3081?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14490584#comment-14490584 ]
ASF GitHub Bot commented on THRIFT-3081: ---------------------------------------- Github user ben-craig commented on a diff in the pull request: https://github.com/apache/thrift/pull/433#discussion_r28189373 --- Diff: lib/cpp/src/thrift/server/TThreadPoolServer.cpp --- @@ -146,9 +73,12 @@ void TThreadPoolServer::serve() { shared_ptr<TProcessor> processor = getProcessor(inputProtocol, outputProtocol, client); // Add to threadmanager pool - shared_ptr<TThreadPoolServer::Task> task( - new TThreadPoolServer::Task(*this, processor, inputProtocol, outputProtocol, client)); - threadManager_->add(task, timeout_, taskExpiration_); + shared_ptr<TConnectedClient> pClient( --- End diff -- I think you are referring to shared_from_this. Here are some of the boost docs talking about the difference: http://www.boost.org/doc/libs/1_57_0/libs/smart_ptr/enable_shared_from_this.html http://www.boost.org/doc/libs/1_57_0/libs/smart_ptr/make_shared.html > C++ Consolidate client processing loops in TServers > --------------------------------------------------- > > Key: THRIFT-3081 > URL: https://issues.apache.org/jira/browse/THRIFT-3081 > Project: Thrift > Issue Type: Improvement > Components: C++ - Library > Affects Versions: 0.8, 0.9, 0.9.1, 0.9.2 > Reporter: James E. King, III > > Currently each of TSimpleServer, TThreadedServer, and TThreadPoolServer have > their own very similar but not quite identical way of processing a client's > lifetime. The code has been copied around and changed; for example a > TThreadPoolServer handles TTransportExceptions from process() differently > than a TThreadedServer does. > There are certain requirements for this processing loop that needs to be met > by every client. Consolidating these three disparate implementations of the > client processing loop into one will provide consistency as well as easier > maintenance, as there will be one common client processing loop that will > contain all the logic from {{eventHandler->createContext}} through > {{client->close}}. > It was also discovered that all three implementations call peek() in each > loop which causes more recv calls than are really needed. Will experiment > with removing peek entirely; expectation is that it is sufficient to have > exception handling around process() and/or have process() return false to end > the processing loop, and peek() is likely an unnecessary temporary band-aid > that got left there. > This was inspired by changes in THRIFT-2441 and I was encouraged to make this > a separate body of work from that change so that it can be reviewed in > isolation from other changes. -- This message was sent by Atlassian JIRA (v6.3.4#6332)