go-zeromq / zmq4

[WIP] Pure-Go implementation of ZeroMQ-4
BSD 3-Clause "New" or "Revised" License
341 stars 56 forks source link

Add context to semaphore #114

Closed dewei-verkada closed 2 years ago

dewei-verkada commented 3 years ago

Currently semaphore will block a qreader read from exiting even when the context have been canceled. From what I can tell its purpose is to guarantee there is a connection first (from addConn) before the read could proceed. However, this requirement makes it hard to write unit tests that might be blocked on waiting for the context to finish.

This change adds context into the semaphore so that read can be unblocked when the context is canceled, instead of resulting in a deadlock.

dewei-verkada commented 2 years ago

@sbinet Could I get a review please?

codecov[bot] commented 2 years ago

Codecov Report

Merging #114 (c9da575) into master (03db79d) will decrease coverage by 0.01%. The diff coverage is 87.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #114      +/-   ##
==========================================
- Coverage   67.36%   67.34%   -0.02%     
==========================================
  Files          29       29              
  Lines        1811     1813       +2     
==========================================
+ Hits         1220     1221       +1     
- Misses        490      491       +1     
  Partials      101      101              
Impacted Files Coverage Δ
pub.go 69.11% <0.00%> (ø)
msgio.go 78.82% <100.00%> (+0.51%) :arrow_up:
router.go 79.79% <100.00%> (ø)
internal/inproc/inproc.go 70.88% <0.00%> (-1.27%) :arrow_down:
socket.go 83.04% <0.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 03db79d...c9da575. Read the comment docs.

sbinet commented 2 years ago

needs https://github.com/go-zeromq/license/pull/9

sbinet commented 2 years ago

thanks a lot (and for sticking with us).

welcome!