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

Add TestParallelHandshake test case #666

Merged

Conversation

Danielius1922
Copy link
Contributor

Handshake are synchronized by a mutex, but there is a panicing scenario when the first handshake fails.

Description

Reference issue

Fixes #...

@Danielius1922
Copy link
Contributor Author

Danielius1922 commented Aug 6, 2024

@Sean-Der: The problematic scenario seems to go like this:

Goroutine A: Gets handshake mutex
Goroutine B: Waits on handshake mutex
Goroutine A: Handshake fails
             close(c.decrypted)
Goroutine B: Gets handshake mutex
             Handshake succeeds or fails
             close(c.decrypted)
             panic - closing of a closed channel

If the first handshake succeeds the issue does not occur.

@Sean-Der Sean-Der force-pushed the adam/bugfix/parallel-handshakes branch from 940f187 to fd4047a Compare August 6, 2024 15:41
Handshake are synchronized by a mutex, but there is a panicing
scenario when the first handshake fails.
@Sean-Der Sean-Der force-pushed the adam/bugfix/parallel-handshakes branch from fd4047a to 38b2704 Compare August 6, 2024 16:07
@Sean-Der Sean-Der marked this pull request as ready for review August 6, 2024 16:07
Copy link

codecov bot commented Aug 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.13%. Comparing base (f3e8a9e) to head (38b2704).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #666      +/-   ##
==========================================
+ Coverage   80.05%   80.13%   +0.07%     
==========================================
  Files         101      101              
  Lines        5355     5355              
==========================================
+ Hits         4287     4291       +4     
+ Misses        697      694       -3     
+ Partials      371      370       -1     
Flag Coverage Δ
go 80.16% <100.00%> (+0.07%) ⬆️
wasm 64.30% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Sean-Der Sean-Der merged commit e20b162 into pion:master Aug 6, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants