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 a crash occuring when trying to ping dead sockets #1407

Merged
merged 3 commits into from Aug 26, 2019

Conversation

@scottinet
Copy link
Member

commented Aug 26, 2019

Description

Fix a rare crash that might occur within the WebSocket entrypoint, when a socket is closed just before a heartbeat occurs. If that happens at just the "right" time, then a heartbeat can start before that socket "close" event is processed and before that socket is marked as dead.

If that happens, pinging that socket results in an exception being thrown. Since it is not caught, this results in an unhandled exception, trapped by Kuzzle and triggering a graceful shutdown.

@scottinet scottinet self-assigned this Aug 26, 2019

@scottinet scottinet added the wip label Aug 26, 2019

@codecov

This comment has been minimized.

Copy link

commented Aug 26, 2019

Codecov Report

Merging #1407 into 1-dev will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##            1-dev    #1407   +/-   ##
=======================================
  Coverage   93.84%   93.84%           
=======================================
  Files         106      106           
  Lines        7393     7393           
=======================================
  Hits         6938     6938           
  Misses        455      455
Impacted Files Coverage Δ
...i/core/entrypoints/embedded/protocols/websocket.js 96.95% <100%> (ø) ⬆️

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 5ccd95b...f14d366. Read the comment docs.

@codecov

This comment has been minimized.

Copy link

commented Aug 26, 2019

Codecov Report

Merging #1407 into 1-dev will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##            1-dev    #1407      +/-   ##
==========================================
+ Coverage   93.84%   93.84%   +<.01%     
==========================================
  Files         106      106              
  Lines        7393     7396       +3     
==========================================
+ Hits         6938     6941       +3     
  Misses        455      455
Impacted Files Coverage Δ
...i/core/entrypoints/embedded/protocols/websocket.js 97% <100%> (+0.05%) ⬆️

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 5ccd95b...ce3a6eb. Read the comment docs.

@scottinet scottinet removed the wip label Aug 26, 2019

@@ -47,7 +47,7 @@
"quotes": [2, "single"],
"semi": [2, "always"],
"space-before-blocks": 2,
"space-in-parens": [2, "never"],
"space-in-parens": [0, "never"],

This comment has been minimized.

Copy link
@Aschen

Aschen Aug 26, 2019

Contributor

Why ? It's because the following wrapping rule ? if ( k === 'something')

This comment has been minimized.

Copy link
@scottinet

scottinet Aug 26, 2019

Author Member

yup

@Aschen
Aschen approved these changes Aug 26, 2019

@scottinet scottinet force-pushed the fix-heartbeat-race-condition branch from 0a70732 to f14d366 Aug 26, 2019

@alexandrebouthinon alexandrebouthinon merged commit cc9500f into 1-dev Aug 26, 2019

6 of 9 checks passed

Header rules No header rules processed
Details
Pages changed 456 new files uploaded
Details
Redirect rules No redirect rules processed
Details
LGTM analysis: JavaScript No new or fixed alerts
Details
Mixed content No mixed content detected
Details
codecov/project 93.84% (+0%) compared to 5ccd95b
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
sonarqube SonarQube reported no issues

@alexandrebouthinon alexandrebouthinon deleted the fix-heartbeat-race-condition branch Aug 26, 2019

Yoann-Abbes added a commit that referenced this pull request Aug 27, 2019
Fix a crash occuring when trying to ping dead sockets (#1407)
Description

Fix a rare crash that might occur within the WebSocket entrypoint, when a socket is closed just before a heartbeat occurs. If that happens at just the "right" time, then a heartbeat can start before that socket "close" event is processed and before that socket is marked as dead.

If that happens, pinging that socket results in an exception being thrown. Since it is not caught, this results in an unhandled exception, trapped by Kuzzle and triggering a graceful shutdown.
Yoann-Abbes added a commit that referenced this pull request Aug 27, 2019
Fix a crash occuring when trying to ping dead sockets (#1407)
Description

Fix a rare crash that might occur within the WebSocket entrypoint, when a socket is closed just before a heartbeat occurs. If that happens at just the "right" time, then a heartbeat can start before that socket "close" event is processed and before that socket is marked as dead.

If that happens, pinging that socket results in an exception being thrown. Since it is not caught, this results in an unhandled exception, trapped by Kuzzle and triggering a graceful shutdown.
@Aschen Aschen referenced this pull request Sep 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.