Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24305 )

Change subject: KUDU-1865 reduce cross-thread allocations in RPC (part 1)
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/24305/4/src/kudu/rpc/connection.cc
File src/kudu/rpc/connection.cc:

http://gerrit.cloudera.org:8080/#/c/24305/4/src/kudu/rpc/connection.cc@801
PS4, Line 801:   // The lambdas below need to hold a reference to the 
connection in case
             :   // it has been closed before the task gets scheduled/aborted.
             :   scoped_refptr<Connection> reffed_this(this);
             :   auto* rpc_error_raw = rpc_error.release();
             :   ReactorTask task{
             :     [=](ReactorThread* rt) {
             :       rt->CompleteConnectionNegotiation(reffed_this.get(),
             :                                         negotiation_status,
             :                                         
unique_ptr<ErrorStatusPB>(rpc_error_raw));
             :     },
             :     [=](const Status& /*s*/) {
             :       
DCHECK(reffed_this->reactor_thread()->reactor()->closing());
             :     },
             :   };
             :   
reactor_thread_->reactor()->ScheduleReactorTask(std::move(task));
> You should create an unique_ptr, then capture it in the lambda by move().
This regular approach isn't working here (yes, even with a mutable lambda).  
The problem stems from the requirement: a boost::function instance must always 
be copyable.  Using a move capture in the lambda with a move-only type variable 
(like std::unique_ptr) makes an instance of boost::function non-copyable, 
resulting in a compiler error.

I addressed the possible memory leak by passing raw pointer and calling delete 
on it in the abort_func member.  Thank you for pointing at that.

Maybe, I'm missing something and you have a recipe to make it work with move 
capture in C++17-based code?  Please let me know.



--
To view, visit http://gerrit.cloudera.org:8080/24305
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I86a2acbd1d8cb724728034c4e91907c99cbfe32e
Gerrit-Change-Number: 24305
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Abhishek Rawat <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Anonymous Coward (763)
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: David Rorke <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Zoltan Martonka <[email protected]>
Gerrit-Comment-Date: Fri, 15 May 2026 15:00:20 +0000
Gerrit-HasComments: Yes

Reply via email to