Skip to content

Fix: size_fd has to be read under a lock #643

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 30, 2024
Merged

Fix: size_fd has to be read under a lock #643

merged 1 commit into from
Jul 30, 2024

Conversation

ldorau
Copy link
Contributor

@ldorau ldorau commented Jul 29, 2024 •
edited
Loading

Description

size_fd has to be read under a lock.

Checklist

  • Code compiles without errors locally
  • All tests pass locally
  • CI workflows execute properly

@ldorau ldorau requested a review from a team as a code owner July 29, 2024 13:43
lukaszstolarczuk
lukaszstolarczuk reviewed Jul 29, 2024
View reviewed changes
Copy link
Contributor

@lukaszstolarczuk lukaszstolarczuk left a comment

Choose a reason for hiding this comment

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

👍

pbalcer
pbalcer approved these changes Jul 29, 2024
View reviewed changes
Copy link
Contributor

@pbalcer pbalcer left a comment

Choose a reason for hiding this comment

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

That's a misleading commit message :) fd_size needs to be read under a lock, not fd_offset (a stack variable) set under one.

lplewa
lplewa reviewed Jul 29, 2024
View reviewed changes
src/provider/provider_os_memory.c
@@ -637,7 +637,8 @@ static inline void assert_is_page_aligned(uintptr_t ptr, size_t page_size) {
static int os_mmap_aligned(void *hint_addr, size_t length, size_t alignment,
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: 12th argument to this function. This screams for refactor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I will do that in a separate PR.

@ldorau ldorau changed the title Fix: fd_offset has to be set under the lock Fix: fd_size has to be read under a lock Jul 29, 2024
@ldorau ldorau changed the title Fix: fd_size has to be read under a lock Fix: size_fd has to be read under a lock Jul 29, 2024
@ldorau ldorau force-pushed the Fix_fd_offset_has_to_be_set_under_the_lock branch from b4eaf0e to 09c089a Compare July 29, 2024 14:43
@ldorau
Copy link
Contributor Author

ldorau commented Jul 29, 2024

That's a misleading commit message :) fd_size needs to be read under a lock, not fd_offset (a stack variable) set under one.

Fixed.

@ldorau ldorau requested a review from lplewa July 29, 2024 14:44
@ldorau ldorau mentioned this pull request Jul 30, 2024
Merged
1 task
@bratpiorka bratpiorka merged commit 41a0f49 into oneapi-src:main Jul 30, 2024
64 checks passed
@ldorau ldorau deleted the Fix_fd_offset_has_to_be_set_under_the_lock branch July 30, 2024 13:23
@lplewa lplewa mentioned this pull request Sep 10, 2024
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukaszstolarczuk lukaszstolarczuk lukaszstolarczuk left review comments

@pbalcer pbalcer pbalcer approved these changes

@bratpiorka bratpiorka bratpiorka approved these changes

@lplewa lplewa Awaiting requested review from lplewa

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ldorau @pbalcer @bratpiorka @lplewa @lukaszstolarczuk