Skip to content

Foundation Classes - Fix bucket array leak in Poly_MergeNodesTool#1336

Open
mbait wants to merge 1 commit into
Open-Cascade-SAS:masterfrom
mbait:fix-poly-mergenodestool-leak
Open

Foundation Classes - Fix bucket array leak in Poly_MergeNodesTool#1336
mbait wants to merge 1 commit into
Open-Cascade-SAS:masterfrom
mbait:fix-poly-mergenodestool-leak

Conversation

@mbait

@mbait mbait commented Jul 11, 2026

Copy link
Copy Markdown

Summary

Poly_MergeNodesTool::MergedNodesMap derives from NCollection_BaseMap but defines no destructor. The base destructor is = default and relies on each subclass calling Destroy() to free the hash bucket arrays (this is what NCollection_DataMap et al. do via Clear(true)), so the bucket array allocated by NCollection_BaseMap::BeginResize() leaks on every Poly_MergeNodesTool instance — i.e. on every STL/OBJ/mesh read that merges nodes (RWStl_Reader, RWObj).

The leaked size scales with mesh size: 2 * nbFacets buckets when the facet count is known, or the 995329-bucket default (~8 MB per import on 64-bit) when it is not. The map nodes live in the handle-held NCollection_IncAllocator and are released correctly — only the raw bucket array leaks.

LeakSanitizer evidence

Reported downstream against prebuilt V8_0_0_p1 Release libs (farfield-ru/libocct-build#1); resolved stack (LSan with fast_unwind_on_malloc=0):

#1 Standard::Allocate(unsigned long)                       libTKernel
#2 NCollection_BaseMap::BeginResize(...)                   libTKernel
#3 Poly_MergeNodesTool::MergedNodesMap::Bind(...)          libTKMath
#4 Poly_MergeNodesTool::PushLastElement(int)               libTKMath
#5 RWStl_Reader::ReadAscii(...)                            libTKDESTL
#6 RWStl_Reader::Read(...)                                 libTKDESTL
#7 RWStl::ReadFile(...)                                    libTKDESTL
#8 StlAPI_Reader::Read(TopoDS_Shape&, char const*)         libTKDESTL

A one-triangle ASCII STL leaks 7,962,640 bytes per StlAPI_Reader::Read() call (the unknown-size default path).

Fix

Add the missing destructor calling Destroy(DataMapNode::delNode, true), matching the idiom used by the other NCollection map classes.

Verification

ASan/LSan-instrumented host performing StlAPI_Reader::Read() + BRepBndLib::Add():

  • before: Direct leak of 7962640 byte(s) in 1 object(s) with the stack above, nonzero exit;
  • after: no LSan report, exit 0.

Also rebuilt Release/Debug/RelWithDebInfo shared-library configurations on Linux (GCC 13) and Windows (MSVC, Ninja) with this change applied on top of V8_0_0_p1 — no build or functional regressions observed in STEP/IGES/STL/OBJ import paths.

Poly_MergeNodesTool::MergedNodesMap derives from NCollection_BaseMap
but defined no destructor. NCollection_BaseMap::~NCollection_BaseMap()
is defaulted and relies on subclasses calling Destroy() to free the
hash bucket arrays (as NCollection_DataMap and the other collection
maps do via Clear(true)), so the bucket array allocated by
BeginResize() leaked on every Poly_MergeNodesTool instance.

The leak is hit on every STL/OBJ/mesh read that goes through
Poly_MergeNodesTool (RWStl_Reader, RWObj) and scales with mesh size:
2 * nbFacets buckets when the facet count is known, or the 995329
bucket default (~8 MB per import on 64-bit) when it is not. The map
nodes themselves live in the handle-held NCollection_IncAllocator and
were released correctly; only the raw bucket array leaked, which is
why LeakSanitizer reports a single direct calloc leak per instance
via Standard::Allocate <- NCollection_BaseMap::BeginResize.

Add the missing destructor calling Destroy(DataMapNode::delNode, true),
matching the idiom used by the other NCollection map classes.

Verified with an AddressSanitizer/LeakSanitizer-instrumented host
performing StlAPI_Reader::Read() + BRepBndLib query: before the fix
LSan reports the direct leak with the stack above; after it the
process exits clean.
@dpasukhi dpasukhi added 2. Bug Something isn't working 1. Foundation Classes Containers, system calls wrappers, smart pointers and other low level of OCCT code 3. CLA waited User need to process with CLA before review or integration processes labels Jul 11, 2026
@dpasukhi

Copy link
Copy Markdown
Member

Dear @mbait thank you for your investigation and detection of leak!

Thank you for your patch. To proceed with integration, please complete signing CLA process:
The links: Contribution Guide
Or https://dev.opencascade.org/get_involved

In case if you already have signed CLA, please share ID that OCCT team shared with your by email after accepting of CLA.

@mbait

mbait commented Jul 14, 2026

Copy link
Copy Markdown
Author

I've submitted the signed CLA, waiting for approval.

@dpasukhi
dpasukhi self-requested a review July 22, 2026 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1. Foundation Classes Containers, system calls wrappers, smart pointers and other low level of OCCT code 2. Bug Something isn't working 3. CLA waited User need to process with CLA before review or integration processes

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

2 participants