should nodeField.reset(); rather than nodeField.release(); #16159

Closed
opened 2026-01-31 04:59:17 +00:00 by claunia · 2 comments
Owner

Originally created by @ltimaginea on GitHub (Dec 16, 2021).

fb597ed304/src/cascadia/TerminalApp/Pane.LayoutSizeNode.cpp (L56-L73)

nodeField.release(); This call does not destroy the managed object, but the unique_ptr object is released from the responsibility of deleting the object. Some other entity must take responsibility for deleting the object at some point. Unfortunately, we lose the pointer to the managed object. So finally, the memory leak will occur. The correct way is nodeField.reset(); . See std::unique_ptr::release .
In my opinion, the most simple and correct solution for Pane::LayoutSizeNode::_AssignChildNode is:

void Pane::LayoutSizeNode::_AssignChildNode(std::unique_ptr<LayoutSizeNode>& nodeField, const LayoutSizeNode* const newNode)
{
	nodeField = newNode ? std::make_unique<LayoutSizeNode>(*newNode) : nullptr;
}
Originally created by @ltimaginea on GitHub (Dec 16, 2021). https://github.com/microsoft/terminal/blob/fb597ed304ec6eef245405c9652e9b8a029b821f/src/cascadia/TerminalApp/Pane.LayoutSizeNode.cpp#L56-L73 `nodeField.release();` **This call does not destroy the managed object**, but the unique_ptr object is released from the responsibility of deleting the object. Some other entity must take responsibility for deleting the object at some point. Unfortunately, we lose the pointer to the managed object. **So finally, the memory leak will occur. The correct way is `nodeField.reset();`** . See [std::unique_ptr::release](http://www.cplusplus.com/reference/memory/unique_ptr/release/) . In my opinion, the most simple and correct solution for `Pane::LayoutSizeNode::_AssignChildNode` is: ```cpp void Pane::LayoutSizeNode::_AssignChildNode(std::unique_ptr<LayoutSizeNode>& nodeField, const LayoutSizeNode* const newNode) { nodeField = newNode ? std::make_unique<LayoutSizeNode>(*newNode) : nullptr; } ```
claunia added the Help WantedIssue-TaskIn-PRProduct-TerminalArea-CodeHealth labels 2026-01-31 04:59:17 +00:00
Author
Owner

@ltimaginea commented on GitHub (Jan 4, 2022):

@zadjii-msft
I replied to you at #11965

@ltimaginea commented on GitHub (Jan 4, 2022): @zadjii-msft I replied to you at #11965
Author
Owner

@ltimaginea commented on GitHub (Feb 23, 2022):

@zadjii-msft @DHowett
Any update on this?

@ltimaginea commented on GitHub (Feb 23, 2022): @zadjii-msft @DHowett Any update on this?
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#16159