Repository navigation
R__memcompress fails with limited stack size #22938
Description
Check duplicate issues.
- Checked for duplicates
Description
R__memcompress crashed from civetweb threads.
Main reason - size of struct bits_internal_state structure approximately 128K which is bigger than default stack size of civetweb thread (approx 100K).
Actually there is compile definition DYN_ALLOC which allows to allocate this memory dynamically,
but it is never used/defined. And even worse. If one enable such definition - allocated memory never will be released while R__lm_free() never called.
Proposed solution - implement R__memcompress_dynamic() with dynamically allocated struct bits_internal_state.
No change of API, no extra work - just new function with same functionality. And of course add everywhere R__lm_free() calls
Reproducer
Compile ROOT with modified flags for RHTTP library.
Add to net/http/CMakeLists.txt, line 75 following statement:
target_compile_definitions(RHTTP PRIVATE -DUSE_STACK_SIZE=102400)
Compile root and run:
[shell] net/http/test/testHttpServer
Problem appears when civetweb compiled in #22731
ROOT version
master, all previous versions as well
Installation method
build from source
Operating system
OpenSUSE
Additional context
No response
Metadata
Metadata
Assignees
Type
Projects
- Status
Activity
Roaimkhan commentedon Jul 27, 2026
@linev From the description, it looks like the stack overflow stems from bits_internal_state being allocated on the thread stack, which exceeds the default CivetWeb thread stack size. I'll investigate introducing a dynamic allocation path (without changing the public API) and ensure all corresponding R__lm_free() calls are handled correctly before opening a PR.
linev commentedon Jul 28, 2026
@Roaimkhan
Looks like this functionality is no longer in production use in ROOT. So in my mind change to dynamic allocation of complete
bits_internal_statestructure should be most simple solution. And I see no reason to activateDYN_ALLOCdefines for that - rather completely dismiss this code. But may be Philippe @pcanal has other ideas.silverweed commentedon Jul 30, 2026
What about allocating the struct as a
thread_local? This way we don't have to free itlinev commentedon Jul 31, 2026
We do not need static variable here - it is purely temporary buffer for zipping.
For me the only question remains - is there any real performance reason to keep this struct in stack.
silverweed commentedon Jul 31, 2026
But if it's just 128 K there's no real disadvantage in having it static (in fact it's arguably better since you don't waste time allocating/deallocating memory every time)
linev commentedon Jul 31, 2026
But
civetwebcan be used with 100 threads and each will preallocate memory - why?In my mind performance of this function is not crucial any longer.
It is specialized function to compress complete buffer at once and not used in the I/O.
It used for
THttpServer, for JSON file compressions and in GL font handling.Just make struct dynamic and thats it
silverweed commentedon Jul 31, 2026
Mine is not mainly a performance concern (the zipping part is probably going to dominate anyway) but a matter of avoiding:
for code simplicity. I don't believe using 128 K of memory per thread is unacceptable even with 100 threads (they'll get deallocated on thread exit anyway), but if you think otherwise go ahead with the dynamic allocation solution.
linev commentedon Jul 31, 2026
PR is here: #22971
It fixes only
R__memcompress(), same can be done withR__zipOld()[zip] allocate dynamically bits_internal_state in R__memcompress
[zip] allocate dynamically bits_internal_state in R__memcompress
[zip] allocate dynamically bits_internal_state in R__memcompress