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

fix(adb/threadpool): remove waitgroups and fix segfault #522

Merged

Conversation

0xNineteen
Copy link
Contributor

@0xNineteen 0xNineteen commented Jan 30, 2025

the main fix is to remove the waitgroups in accounts-db loading/validating so we can have errors propagate correctly. while doing this, i also noticed a bug in the homo-threadpool impl where addOne to existing tasks will cause re-allocs on existing thread state leading to a segfault. there was also some code in repair which would join the threadpool after a single schedule in a loop, which i assumed was a bug and fixed as well.

there was also a bug which tsan found where an allocator was used in the registry without a lock which i fixed as well.

this lead to another bug in tsan about registry init race conditions which was fixed in 553c803.

What changed?

  • Refactored spawnThreadTasks to use a homogeneous thread pool instead of raw thread spawning
  • Removed single-threaded code paths in favor of unified multi-threaded implementation
  • Modified loadFromSnapshot to return void instead of duration
  • Adjusted testnet accounts per file estimate from 1000 to 500
  • Added thread pool capacity pre-allocation to avoid reallocation during scheduling
  • Added new test case for spawnThreadTasks

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@0xNineteen 0xNineteen marked this pull request as ready for review January 30, 2025 13:07
@0xNineteen 0xNineteen requested review from InKryption and dnut January 30, 2025 13:07
@0xNineteen 0xNineteen self-assigned this Jan 30, 2025
@0xNineteen 0xNineteen force-pushed the 01-30-fix_adb_threadpool_remove_waitgroups_and_fix_segfault branch from 34f99cb to a8625a2 Compare January 30, 2025 16:09
Copy link
Contributor

@InKryption InKryption left a comment

Choose a reason for hiding this comment

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

Two very minor comments, overall a solid improvement and fix.

src/prometheus/registry.zig Outdated Show resolved Hide resolved
src/utils/thread.zig Show resolved Hide resolved
InKryption
InKryption previously approved these changes Jan 30, 2025
src/shred_network/repair_service.zig Show resolved Hide resolved
@0xNineteen 0xNineteen enabled auto-merge February 3, 2025 20:03
@0xNineteen 0xNineteen added this pull request to the merge queue Feb 3, 2025
Merged via the queue into main with commit 645fe1a Feb 3, 2025
12 of 13 checks passed
@0xNineteen 0xNineteen deleted the 01-30-fix_adb_threadpool_remove_waitgroups_and_fix_segfault branch February 3, 2025 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants