feat(sqlite): make thread_stack_size optional with safe defaults #4080
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Changes `thread_stack_size` from `usize` to `Option<usize>` to address concerns about safety and platform compatibility. Key improvements: - Default to `None`, using Rust std's default stack size (typically 2MB) - Only apply custom stack size when explicitly configured - Safer for user callbacks with unpredictable stack requirements - Platform-agnostic (handles 32-bit vs 64-bit differences automatically) - Marked as an advanced option in documentation with appropriate warnings This addresses the feedback from PR launchbadge#3885 about hardcoded stack sizes being unsafe due to: 1. Unpredictable stack needs of user-supplied callbacks 2. Platform-specific requirements (32-bit vs 64-bit) 3. Need for conservative defaults Related: launchbadge#3885
Collaborator
| @ZenasSong please run |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Changes
thread_stack_sizefromusizetoOption<usize>to address concerns about safety and platform compatibility.Key improvements:
None, using Rust std's default stack size (typically 2MB)This addresses the feedback from PR #3885 about hardcoded stack sizes being unsafe due to:
Related: #3885
Does your PR solve an issue?
Delete this text and add "fixes #(issue number)".
Do not just list issue numbers here as they will not be automatically closed on merging this pull request unless prefixed with "fixes" or "closes".
Is this a breaking change?
Delete this text and answer yes/no and explain.
If yes, this pull request will need to wait for the next major release (
0.{x + 1}.0)Behavior changes can be breaking if significant enough.
Consider Hyrum's Law: