[ 
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)

Reply via email to