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 race condition #68

Merged
merged 1 commit into from
Oct 26, 2020
Merged

Fix race condition #68

merged 1 commit into from
Oct 26, 2020

Conversation

nkoenig
Copy link
Contributor

@nkoenig nkoenig commented Oct 16, 2020

A segfault can occur if a websocket disconnects while the websocket server is handling a new ign-transport message.

The addition of std::lock_guard<std::mutex> mainLock(this->subscriptionMutex); resolves the problem, and the extra if (this->connections.find(socketId) != this->connections.end()) is just for good measure.

Signed-off-by: Nate Koenig [email protected]

Signed-off-by: Nate Koenig <[email protected]>
@codecov
Copy link

codecov bot commented Oct 16, 2020

Codecov Report

Merging #68 into ign-launch1 will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           ign-launch1      #68   +/-   ##
============================================
  Coverage        29.01%   29.01%           
============================================
  Files                3        3           
  Lines              734      734           
============================================
  Hits               213      213           
  Misses             521      521           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb93a00...8ce17f8. Read the comment docs.

@nkoenig nkoenig merged commit 4538f98 into ign-launch1 Oct 26, 2020
@nkoenig nkoenig deleted the websocket_race_condition branch October 26, 2020 18:53
@mjcarroll mjcarroll mentioned this pull request Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📜 blueprint Ignition Blueprint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants