Close app

[ovv]

• Remove py34 from tox
• Fix event loop for py35 tests with tox (not sure why it was working with Travis)
• Create a client and a server app in test. One app being its own server/client was bugged.
• Test for Not Implemented and Server Internal Error response

[ovv]

@vodik I think it's easier to have the test use two app (one client, one server).

With only one the server connection is closed first due to dict being ordered in 3.6 and this is not working in TCP

[codecov-io]

Codecov Report

Merging #33 into master will decrease coverage by 0.1%. The diff coverage is 73.33%.

@@            Coverage Diff            @@
##           master     #33      +/-   ##
=========================================
- Coverage   63.91%   63.8%   -0.11%     
=========================================
  Files          16      16              
  Lines         934     945      +11     
  Branches      142     143       +1     
=========================================
+ Hits          597     603       +6     
- Misses        278     286       +8     
+ Partials       59      56       -3

Impacted Files	Coverage Δ
aiosip/pytest_plugin.py	88.57% <100%> (ø)	:arrow_up:
aiosip/application.py	75.43% <100%> (+1.1%)	:arrow_up:
aiosip/dialog.py	59.12% <33.33%> (-0.14%)	:arrow_down:
aiosip/connections.py	78.26% <75%> (-11.22%)	:arrow_down:
aiosip/dialplan.py	73.07% <0%> (+3.84%)	: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 f520cef...a909229. Read the comment docs.

[vodik]

@ovv I didn't intend to have a single application and noticed last night. I was actually working on fixing it. Well you beat me and this looks awesome.

[vodik]

So I'm merging this because last night I think I found an issue in the connection subsystem, specifically how create_dialog/dispatch can interact with each other and create multiple connections with duplicate protocol instances, and I think that is still the case with this patch set.

[ovv]

I was intrigued by your comment on the other PR so I took a stab at fixing it :)

create multiple connections with duplicate protocol

Well obviously it shouldn't. Are you sure it was the same connection ?

When using only one app we had two connection one for each side with the local_addr and remote_addr inverted.

[vodik]

What happens is that create_dialogrun creates a connection with only local_addr set. A request is sent out on that connection, and when a response returns, this logic fires:

connection = self._connections.get((local_addr, remote_addr, type(protocol)))
if not connection:
    LOG.debug('New connection for %s', remote_addr)
    connection = Connection(local_addr, remote_addr, protocol, self)
    self._connections[local_addr, remote_addr, type(protocol)] = connection

Well, the problem is now there's now a remote_addr, so _connections.get returns None and a new connection is created. However protocol is passed into the dispatch method, and already uniquely identifies the connection. The problem is in how connection bookkeeping is done.

After all, with UDP, we don't care about remote_addr per say, just local_addr because we use sendto.

To highlight the problem, I added a __repr__ to the connection class and dump out the set of connections on close:

[<Connection to=None, from=127.0.0.1:6000, 0x7f2cae97db70>,
 <Connection to=127.0.0.1:7000, from=127.0.0.1:6000, 0x7f2cae97db70>]

Two connection, different remote_addr, same protocol instance.

I hope this makes sense.

EDIT: changes the to/from in the repr because I had the labels flipped. This is what I get for hacking on something half asleep. remote_addr was perviously bound to from= field...

[vodik]

Okay, as a heads up, I'm currently working on decoupling connections from the application entirely in the style of aiohttp Connectors. This should let us better handle the quirks here.

[vodik]

Changes in #35 address the problem, if you don't mind giving it a look over before I clean it up.