Create common echo handler

[ocaballeror]

Fixes #333

The issue was that the socket was not being removed from the poller's list before being closed, so the next time the poller tried to check on its sockets, the closed ones would still be there and that would cause a connection error.

I don't know why this is needed on Windows but not on Linux, but this should fix the problem.

By the way, I can't believe we didn't have a test where we close a connected socket yet 🤷‍♂ .

[codecov[bot]]

Codecov Report

Merging #337 into master will decrease coverage by <.01%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #337      +/-   ##
==========================================
- Coverage   99.26%   99.26%   -0.01%     
==========================================
  Files          26       26              
  Lines        3549     3548       -1     
  Branches      257      257              
==========================================
- Hits         3523     3522       -1     
  Misses         14       14              
  Partials       12       12

Impacted Files	Coverage Δ
osbrain/tests/common.py	100% <100%> (ø)	:arrow_up:
osbrain/tests/test_agent_serialization.py	100% <100%> (ø)	:arrow_up:
osbrain/tests/test_agent.py	100% <100%> (ø)	:arrow_up:
osbrain/tests/test_agent_req_rep.py	100% <100%> (ø)	:arrow_up:

---

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 2ed8aee...2ec72a2. Read the comment docs.

[Peque]

Thanks for this PR! :blush:

We could split it into:

1. Create the new echo_handler and use it in existing tests (I would merge that PR right away)
2. Consider adding the new test with the fix

About the latter... would you mind sharing the error you get on Windows? Alternatively, would you mind splitting the commits into test+fix to trigger AppVeyor's pipelines and share the traceback that way?

.close_all() should be a safe call, which means that it should be executed by the main thread in a running agent. The main thread is the one that should be polling, so there should be no conflict. Another thread should not try to close sockets as socket operations are not thread-safe.

That does not mean that we should not be unregistering sockets from the poller anyways. Specially if that is not automated on socket close by ZeroMQ.

[Peque]

@ocaballeror I took a look at this and I woud rather propose https://github.com/opensistemas-hub/osbrain/pull/338. I used your fix commit (maintaining authorship) but changed the test to something I think it is more appropriate.

What do you think?

[ocaballeror]

Sure, let me repurpose this PR for just the echo handler and continue the discussion on the new one.