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

General Cleanup #539

Merged
merged 4 commits into from
Mar 4, 2024
Merged

General Cleanup #539

merged 4 commits into from
Mar 4, 2024

Conversation

sean-parent
Copy link
Member

  • Some lint fixes for CodeQL (used in the chain library CI).
  • Cleaned up mutex usage in await() to eliminate a flag and directly protect the element being modified.
  • Fixed a memory leak in the Windows default executor and a potential leak if scheduling work fails.
  • Made package_task<> move-only, simplifying promise by removing the promise count.
  • Removed unused reduction_failed error code.
  • Added a reduction test for future<future>

- Cleaned up mutex usage in await to eliminate a flag, and directly protect the element being modified.
- Fixed a memory leak in Windows default executor and a potential leak if scheduling work fails.
- Made package_task<> move-only which simplifies promise by removing promise count.
- Removed unused reduction_failed error code.
- Added a reduction test for future<future<void>>

dispatch_group_async_f(detail::group()._group,
dispatch_get_global_queue(platform_priority(P), 0),
new f_t(std::move(f)), [](void* f_) {
new f_t(std::foward<F>(f)), [](void* f_) {
Copy link
Member

Choose a reason for hiding this comment

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

Typo should be forward.

The task pointer was already being deleted by a unique pointer after execution. I kept the code (with a typo fix) to ensure it is deleted in case of failure.
@sean-parent sean-parent merged commit facbd55 into main Mar 4, 2024
6 checks passed
@sean-parent sean-parent deleted the sean-parent/codeql-fixes branch March 4, 2024 19:44
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