Skip to content

Fix unsafe accesses of host objects from the manager#2248

Merged
stevenengler merged 3 commits intoshadow:mainfrom
stevenengler:connectivity-info
Jul 1, 2022
Merged

Fix unsafe accesses of host objects from the manager#2248
stevenengler merged 3 commits intoshadow:mainfrom
stevenengler:connectivity-info

Conversation

@stevenengler
Copy link
Copy Markdown
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 requested a review from sporksmith June 29, 2022 21:42
@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
@stevenengler stevenengler requested a review from sporksmith July 1, 2022 00:48
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
Copy Markdown
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 enabled auto-merge July 1, 2022 18:23
@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.

2 participants