Skip to content

test(os): join threads to avoid race/use-after-free in osSemaphoreTests#35055

Merged
guanshengliang merged 2 commits intomainfrom
fix/citest/osSemaphore
Apr 7, 2026
Merged

test(os): join threads to avoid race/use-after-free in osSemaphoreTests#35055
guanshengliang merged 2 commits intomainfrom
fix/citest/osSemaphore

Conversation

@facetosea
Copy link
Copy Markdown
Contributor

Description

Issue(s)

  • Close/close/Fix/fix/Resolve/resolve: Issue Link

Checklist

Please check the items in the checklist if applicable.

  • Is the user manual updated?
  • Are the test cases passed and automated?
  • Is there no significant decrease in test coverage?

Copilot AI review requested due to automatic review settings April 3, 2026 07:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates osSemaphoreTests to avoid races/use-after-free by no longer detaching producer/waiter threads that still reference stack-allocated semaphores.

Changes:

  • Replace std::thread(...).detach() with named std::thread variables.
  • Join all spawned threads before destroying the semaphore instances.
  • Apply the same pattern across functional and performance-oriented semaphore tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread source/os/test/osSemaphoreTests.cpp Outdated
Comment thread source/os/test/osSemaphoreTests.cpp Outdated
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request improves the reliability of the semaphore tests by replacing detached threads with named thread objects that are explicitly joined. This ensures that background threads finish their execution before the semaphores they reference are destroyed. The review feedback correctly identifies unnecessary variable captures in lambda expressions within the performance tests, which should be removed to improve code clarity.

Comment thread source/os/test/osSemaphoreTests.cpp Outdated
Comment thread source/os/test/osSemaphoreTests.cpp Outdated
Copy link
Copy Markdown
Contributor

@dapan1121 dapan1121 left a comment

Choose a reason for hiding this comment

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

LGTM

@guanshengliang guanshengliang merged commit 0c15fde into main Apr 7, 2026
11 of 12 checks passed
@facetosea facetosea deleted the fix/citest/osSemaphore branch April 20, 2026 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants