Skip to content
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.

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

DISPATCH-2039 Add option -DQD_DISABLE_MEMORY_POOL for CMake build #110

Merged

Conversation

jiridanek
Copy link
Contributor

No description provided.

@jiridanek jiridanek force-pushed the jd_2022_03_01_disable_mempool branch 3 times, most recently from e27d7da to 89f8d9f Compare March 2, 2022 20:49
@jiridanek jiridanek changed the title DISPATCH-2039 WIP disable free_lists to see what happens DISPATCH-2039 Add option -DQD_DISABLE_MEMORY_POOL for CMake build Mar 2, 2022
@jiridanek jiridanek marked this pull request as ready for review March 2, 2022 20:49
@jiridanek jiridanek requested a review from kgiusti March 2, 2022 20:50
@jiridanek jiridanek force-pushed the jd_2022_03_01_disable_mempool branch from 6fa2d95 to ec3fdf8 Compare March 4, 2022 19:36
# Safe pointers require memory pool to work (memory may never be relinquished to the OS, otherwise safe pointers may
# break). Therefore, only disable memory pool for special debugging purposes. Sanitizers minimize memory reuse and
# make it significantly less likely that a safe pointer breaks.
set(QD_DISABLE_MEMORY_POOL OFF CACHE STRING "Disables memory pool. Should be only used with asan or msan RUNTIME_CHECK")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jiridanek any chance this patch can for the 'cmake' command to fail if QD_DISABLE_MEMORY_POOL is set but RUNTIME_CHECK is not asan/msan?

Issue a message(FATAL_ERROR... like done elsewhere in this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess so, this should thwart the unwarry who might want to compile production build with the option, and put them back on the right path. Not that they would not be taught differently soon enough, the safe pointers break a lot, without sanitizers. (increasing the quarantine_size_mb sanitizer option helps make the router more stable).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added in a fixup

Copy link
Contributor

@kgiusti kgiusti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment (optional suggestion), otherwise LGTM

@jiridanek jiridanek force-pushed the jd_2022_03_01_disable_mempool branch from ec3fdf8 to 740b49d Compare March 9, 2022 14:54
@jiridanek jiridanek force-pushed the jd_2022_03_01_disable_mempool branch from 740b49d to c1fb45f Compare March 9, 2022 15:33
@jiridanek jiridanek merged commit 84f664b into skupperproject:main Mar 9, 2022
@jiridanek jiridanek deleted the jd_2022_03_01_disable_mempool branch March 9, 2022 16:19
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.

2 participants