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: read goroutine nil pointer panic #235

Merged
merged 7 commits into from
Jul 24, 2021
Merged

fix: read goroutine nil pointer panic #235

merged 7 commits into from
Jul 24, 2021

Conversation

Verthandii
Copy link
Contributor

1. Are you opening this pull request for bug-fixs, optimizations or new feature?

bug-fixs

2. Please describe how these code changes achieve your intention.

windows下的 listenerRun 也就是 read 操作是一个 goroutine,而另外的 react & write 在另一个 goroutine,并且 write 出错时可能会修改 conn,这个操作必须让 read goroutine 知道,否则会导致 panic,所以做了此修改,

3. Please link to the relevant issues (if any).

4. Which documentation changes (if any) need to be made/updated because of this PR?

4. Checklist

  • I have squashed all insignificant commits.
  • I have commented my code for explaining package types, values, functions, and non-obvious lines.
  • I have written unit tests and verified that all tests passes (if needed).
  • I have documented feature info on the README (only when this PR is adding a new feature).
  • (optional) I am willing to help maintain this change if there are issues with it later.

eventloop_windows.go Show resolved Hide resolved
acceptor_windows.go Outdated Show resolved Hide resolved
acceptor_windows.go Outdated Show resolved Hide resolved
eventloop_windows.go Outdated Show resolved Hide resolved
errors/errors.go Outdated
@@ -41,6 +41,8 @@ var (
ErrUnsupportedUDSProtocol = errors.New("only unix is supported")
// ErrUnsupportedPlatform occurs when running gnet on an unsupported platform.
ErrUnsupportedPlatform = errors.New("unsupported platform in gnet")
// ErrConnectionClosed occurs when connection is closed but eventloop.ch still receive this connection.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

改成 "ErrConnectionClosed occurs when the event-loop receives a closed connection."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link
Owner

@panjf2000 panjf2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@panjf2000 panjf2000 added the pending merged This PR has been reviewed and approved label Jul 24, 2021
@codecov
Copy link

codecov bot commented Jul 24, 2021

Codecov Report

Merging #235 (3db9253) into master (bc618a4) will decrease coverage by 0.14%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #235      +/-   ##
==========================================
- Coverage   82.50%   82.35%   -0.15%     
==========================================
  Files          18       18              
  Lines        1743     1746       +3     
==========================================
  Hits         1438     1438              
- Misses        253      255       +2     
- Partials       52       53       +1     
Flag Coverage Δ
unittests 82.35% <33.33%> (-0.15%) ⬇️

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

Impacted Files Coverage Δ
eventloop_windows.go 87.50% <0.00%> (-1.52%) ⬇️
acceptor_windows.go 100.00% <100.00%> (ø)

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 bc618a4...3db9253. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending merged This PR has been reviewed and approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants