Skip to content

Fix server CallData leak by not pinning lifetime on AsyncNotifyWhenDone #526

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
twaltersp wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix server CallData leak by not pinning lifetime on AsyncNotifyWhenDone #526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2f438c2
Fix server CallData leak by not pinning lifetime on AsyncNotifyWhenDone
twaltersp Aug 28, 2026
ca1abe3
Memory leak in client if error occurs causing CompleteClientUnaryCall…
todd-walter-ai Aug 29, 2026
9fd85b0
Update event_data.cc
twaltersp Aug 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified ...erver Template/RPC Service/ServiceName/RPC Methods/Set package_service_method Response.vi
View file
Open in desktop
Binary file not shown.
Binary file modified labview source/gRPC lv Support/Client API/Client Complete Client Streaming Call.vim
View file
Open in desktop
Binary file not shown.
Binary file modified labview source/gRPC lv Support/Client API/Client Unary Call.vim
View file
Open in desktop
Binary file not shown.
66 changes: 13 additions & 53 deletions src/event_data.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@ namespace grpc_labview
{
auto callData = std::shared_ptr<CallData>(new CallData(server, service, cq));

auto finishedTag = new CallFinishedTag(callData);
callData->_callFinishedTag = finishedTag;
callData->_ctx.AsyncNotifyWhenDone(finishedTag);
// Do not call AsyncNotifyWhenDone. Holding CallData on that CQ tag
// leaked one object per RPC because the tag is not reliably delivered
// for AsyncGenericService. Instead, rely on the existing CompletionQueueTag
// to keep CallData alive only until stream.Finish() is delivered on the CQ
// (after LabVIEW has unregistered the CallData pointer via CloseServerEvent).
// Read/Write.

// Start the state machine which waits for a new call to arrive.
auto tag = new CompletionQueueTag(callData);
Expand Down Expand Up @@ -68,14 +71,16 @@ namespace grpc_labview
//---------------------------------------------------------------------
bool CallData::IsCancelled()
{
return _ctx.IsCancelled();
// Unsafe to call _ctx.IsCancelled() without a completed
// AsyncNotifyWhenDone tag. See CallData::Create.
return false;
Comment thread
twaltersp marked this conversation as resolved.
}

//---------------------------------------------------------------------
//---------------------------------------------------------------------
bool CallData::IsActive()
{
return _status != CallStatus::Finished && _status != CallStatus::Finishing && !IsCancelled();
return _status != CallStatus::Finished && _status != CallStatus::Finishing;
}

//---------------------------------------------------------------------
Expand Down Expand Up @@ -134,18 +139,8 @@ namespace grpc_labview

if (!ok && _status != CallStatus::Finished)
{
if (_status == CallStatus::WaitingForConnection)
{
// Ugh. When using the grpc async APIs, you are required to call AsyncNotifyWhenDone if you want to call IsCancelled
// on the ServerContext. However, the tag registered with AsyncNotifyWhenDone is only notified if a RPC call actually
// starts, and you must call AsyncNotifyWhenDone before the call starts or tag will not be notified either. Generally,
// it is acceptable to just leak this one tag on server shutdown. However, because we maintain a shared pointer to the
// server, we will end up leaking everything if we don't clean up this tag. As a work around, we delete the tag here.
// This leaves a dangling tag pointer in the completion queue, but it never does anything with the tag. It only delivers
// the tag from the Next call which we know will never be triggered so this should be safe.
delete _callFinishedTag;
_callFinishedTag = nullptr;
}
// RequestCall ok=false means the call never started (typically
// CQ shutdown). Finish completing with ok=false is handled below.
_status = CallStatus::Finishing;
}

Expand Down Expand Up @@ -211,41 +206,6 @@ namespace grpc_labview
_stream.Finish(_callStatus, new CompletionQueueTag(shared_from_this()));
}

//---------------------------------------------------------------------
//---------------------------------------------------------------------
void CallData::FinishFromCompletionQueue()
{
std::lock_guard<std::mutex> lock(_stateMutex);

_callFinishedTag = nullptr;

// The call was completed normally from LV code.
if (_status == CallStatus::Finishing || _status == CallStatus::Finished)
{
return;
}

// If FinishFromCompletionQueue is called and we are not already finishing because the user completed
// the call from LV, then it means either the server is shutting down or the call was cancelled. In either
// case there is no point in calling Finish on the stream so just mark the call as finished.
_status = CallStatus::Finished;
}

//---------------------------------------------------------------------
//---------------------------------------------------------------------
CallFinishedTag::CallFinishedTag(std::shared_ptr<CallData> callData)
{
_callData = callData;
}

//---------------------------------------------------------------------
//---------------------------------------------------------------------
void CallFinishedTag::Proceed(bool ok)
{
_callData->FinishFromCompletionQueue();
delete this;
}

//---------------------------------------------------------------------
//---------------------------------------------------------------------
ReadNextTag::ReadNextTag(std::shared_ptr<CallData> callData) :
Expand Down Expand Up @@ -328,4 +288,4 @@ namespace grpc_labview
{
serverStartStatus = 0;
}
}
}
15 changes: 0 additions & 15 deletions src/grpc_server.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ namespace grpc_labview
class LabVIEWgRPCServer;
class LVMessage;
class CallData;
class CallFinishedTag;
class MessageElementMetadata;
struct MessageMetadata;

Expand Down Expand Up @@ -123,7 +122,6 @@ namespace grpc_labview
void Proceed(bool ok) override;
bool Write(int8_t* cluster);
void FinishFromLabVIEW();
void FinishFromCompletionQueue();
bool IsCancelled();
bool IsActive();
bool ReadNext(int8_t* cluster);
Expand All @@ -141,7 +139,6 @@ namespace grpc_labview
grpc::GenericServerAsyncReaderWriter _stream;
grpc::ByteBuffer _rb;
grpc::Status _callStatus;
CallFinishedTag* _callFinishedTag;

std::shared_ptr<LVMessage> _request;
std::shared_ptr<LVMessage> _response;
Expand All @@ -159,18 +156,6 @@ namespace grpc_labview
CallStatus _status;
};

//---------------------------------------------------------------------
//---------------------------------------------------------------------
class CallFinishedTag : public CallDataBase
{
public:
CallFinishedTag(std::shared_ptr<CallData> callData);
void Proceed(bool ok) override;

private:
std::shared_ptr<CallData> _callData;
};

//---------------------------------------------------------------------
// Completion queue tag that keeps CallData alive
//---------------------------------------------------------------------
Expand Down
Loading