Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 30, 2026

Addresses data race where blockQueueSize was documented as mutex-protected but accessed atomically, creating potential for incorrect assumptions about thread safety.

Changes

  • Moved blockQueueSize and blockQueueLen from mutex-protected section to atomic variables section in Client struct
  • Ensures proper 64-bit alignment required for atomic operations
  • Documents actual access pattern (all existing accesses were already atomic)
type Client struct {
    // variables used in atomic operations (declared first for alignment)
    msgNum              uint32
    sendworkers         int32
    transmittedBlocks   int32
    skippedBlocks       int32
+   blockQueueLen       int32  // atomic access
    WriteData           int64
    WriteDataCompressed int64
+   blockQueueSize      int64  // atomic access
    
    // mutex protected
    dispatchMutex  sync.Mutex
-   blockQueueSize int64  // incorrect - actually atomic
-   blockQueueLen  int32  // incorrect - actually atomic
    blockQueueMap  map[Byte128]*blockQueueEntry
    blockQueueList []*blockQueueEntry

All access points (queue size checks, compression adjustments, stats) already use atomic operations correctly.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits January 30, 2026 17:01
Co-authored-by: fredli74 <6632456+fredli74@users.noreply.github.com>
Co-authored-by: fredli74 <6632456+fredli74@users.noreply.github.com>
Copilot AI changed the title [WIP] Update to address feedback on Hashbox v0.8 review comments Fix blockQueueSize data race by moving to atomic section Jan 30, 2026
Copilot AI requested a review from fredli74 January 30, 2026 17:04
@fredli74 fredli74 closed this Jan 30, 2026
@fredli74 fredli74 deleted the copilot/sub-pr-2-again branch January 30, 2026 20:26
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