Skip to content

core: use ::operator delete for RVec heavy storage on Windows#22460

Open
wacfrr wants to merge 1 commit into
root-project:masterfrom
wacfrr:fix-rvec-win32-delete
Open

core: use ::operator delete for RVec heavy storage on Windows#22460
wacfrr wants to merge 1 commit into
root-project:masterfrom
wacfrr:fix-rvec-win32-delete

Conversation

@wacfrr
Copy link
Copy Markdown

@wacfrr wacfrr commented Jun 2, 2026

This Pull request:

Fixes a cross-module/cross-CRT heap allocation mismatch in RVec on Windows platforms. This issue can lead to undefined behaviors, memory corruption, or silent exits when memory allocated inside RVec is freed across module boundaries (e.g., between a pre-compiled DLL and JIT-generated code).

Changes or fixes:

  • math/vecops/src/RVec.cxx: In SmallVectorBase::grow_pod, replaced malloc/realloc/free with ::operator new(std::nothrow) and ::operator delete, guarded by #ifdef _WIN32.

  • math/vecops/inc/ROOT/RVec.hxx: In SmallVectorTemplateBase::grow and destructor-related deallocations, applied the same guarded replacement to ensure all heap operations are unified across the class layout.

These changes are strictly restricted to Windows via platform macros; Linux and macOS targets remain completely untouched to preserve their high-performance native realloc paths.

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

This PR fixes #22449

@wacfrr wacfrr requested a review from dpiparo as a code owner June 2, 2026 15:51
@jblomer
Copy link
Copy Markdown
Contributor

jblomer commented Jun 2, 2026

This will need a matching change in the RNTuple treatment of RVecs

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 3, 2026

Test Results

    22 files      22 suites   3d 11h 8m 43s ⏱️
 3 864 tests  3 864 ✅ 0 💤 0 ❌
76 236 runs  76 236 ✅ 0 💤 0 ❌

Results for commit d86cdb5.

@wacfrr
Copy link
Copy Markdown
Author

wacfrr commented Jun 3, 2026

/format

Hi, thanks for triggering the CI!

I have just run the local test suites for both math and RNTuple components on my Windows environment (ctest -C RelWithDebInfo). Fortunately, all tests passed successfully (154/154 passed for math, and all RNTuple test targets passed as well). It seems the basic routines and standard behaviors are working well with this patch.

Regarding the clang-format failure on the CI, I apologize for the noise. My local formatter automatically touched some surrounding legacy code lines, which caused the style check to fail.

I've left a /format command above to see if the project's formatting bot can automatically clean it up. If it doesn't support automatic fixing, please let me know, and I will manually revert the formatting changes on the untouched legacy code lines.

Let's monitor the rest of the CI matrix results to see if there are any other hidden regressions. Thanks again!

I understand that this is a critical modification involving core behaviors, and I would be more than happy to continue our discussion on how to properly align this with the rest of the ecosystem.

@bellenot
Copy link
Copy Markdown
Member

bellenot commented Jun 3, 2026

@wacfrr it seems to be working for df002_dataModel.C. There are some other test failures I want to check before going further...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Windows] Macro df002_dataModel.C crashes on exit or exits silently due to potential RVec/Heap issues across DLL boundaries in Cling JIT

3 participants