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 unsafe accesses of host objects from the manager #2248

Merged
merged 3 commits into from
Jul 1, 2022

Conversation

stevenengler
Copy link
Contributor

@stevenengler stevenengler commented Jun 29, 2022

The manager accesses Host objects without acquiring the hosts' locks, which is not safe.

Edit:

The simulation results have changed slightly in this PR, so I will try to investigate where this difference is from.

1656599789_grim

Edit 2:

With the workaround commit, the results are the same. We can revert that commit in a new PR.

1656636436_grim

@stevenengler stevenengler self-assigned this Jun 29, 2022
@github-actions github-actions bot added the Component: Main Composing the core Shadow executable label Jun 29, 2022
@stevenengler stevenengler changed the title Fixed unsafe accesses of host objects from the manager Fix unsafe accesses of host objects from the manager Jun 30, 2022
@stevenengler stevenengler removed the request for review from sporksmith June 30, 2022 14:50
src/main/core/manager.c Outdated Show resolved Hide resolved
This fixes a memory safety issue where `manager_getNodeBandwidth{Up,Down}()`
accesses a `Host` object without acquiring its lock. We cannot acquire the lock
here because a `GMutex` is not reentrant.
This fixes a memory safety issue where `manager_getLatency()` and
`manager_getReliability()` access a `Host` object without acquiring its lock.
We cannot acquire the lock here because a `GMutex` is not reentrant.
This will noticably change the networking performance of larger simulations.
@stevenengler
Copy link
Contributor Author

Some examples of the expected network performance change (the difference between PR #2248 and nightly):

transfer_time_51200 exit

transfer_time_5242880 exit

client_goodput_5MiB exit

@stevenengler stevenengler merged commit be76765 into shadow:main Jul 1, 2022
@stevenengler stevenengler deleted the connectivity-info branch July 1, 2022 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Main Composing the core Shadow executable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants