[BUG] Fixed: Detect reusable bindings and binding conflicts correctly

[ethouris]

Fixes #1503

The problem: the original procedure for finding a binding was only checking if the attempted binding can be matched with some other with the same port number. If found, it was reused, if settings allowed, otherwise a new binding was created. This was completely wrong because it didn't take into account a possibility that an existing binding may conflict with attempted binding and make it impossible to set up, as well as it was happening to reuse the binding that didn't exactly match the requested specification. The binding conflict would still result in error in the system ::bind call in case when the multiplexer was decided to not be reused, but that was also misleading.

Binding to 0.0.0.0:5000 could be allowed to be reused, if the original binding was REUSEADDR and if the attempted binding is also 0.0.0.0:5000. However, such a binding makes the port 5000 busy for all IPv4 addresses, so attempt to bind to 127.0.0.1:5000 is impossible to set up, as the port 5000 is busy, as well as you cannot pretend that the binding to 0.0.0.0:5000 covers this binding requirement. This situation wasn't so far handled at all, so there wasn't introduced any error reporting that situation.

Similarly, if you attempt to bind to 1.2.3.4:5000 and such a binding is already found, then you can only go two ways:

• "wink in" this binding, if you have the same UDP-related settings and it's generally allowed to be reused
• if this binding cannot be "winked in", this may only result in conflict.

Likely this attempt to create a binding could still fail, when the CChannel object is created and the inside UDP socket will be then tried to be bound to this address, and the conflict between either two the same IP and port sets or a specified IP would conflict with a wildcard, but then this would be reported misleadingly through the system bind function. The problem is that this error would be a consequence of rejection of binding reusage.

Beside this "relying on system error", however, this procedure was also conaining an error by allowing that an existing binding to given port be reused as long as it has correct UDP-related settings and the same port, "winking in" an existing binding that didn't carry the same setup.

The new approach considers the following: the existing multiplexers are searched for an intersecting binding, that is, one that covers the same pool of addresses, at least partially.

If no intersecting binding was found, a new multiplexer is created for that socket.

If found, IP addresses must be identical (plus the IPV6ONLY flag) and additionally both must have REUSEADDR=true and identical UDP settings (as it was before). If an intersecting multiplexer was found, but not identical, it results in a conflict.

[maxsharabayko]

CI build fails.

[alexpokotilo]

According to conversation #1513
these statements:

Here the procedure has been changed to check if:

1. You have a binding with the same port.
2. If so, whether this is a binding that covers the range of the attempted binding
3. If the found binding is reusable, it may only result in reusage, or in conflict.
4. Only if the binding is not reusable does it proceed to create a new binding.

Should be changed to:

1. if you try to bind ip:port and there is multiplexer exists with the same port and requested ip to bind intersects but not exactly match with ip of this existing multiplexer:

• if this multiplexer exists without any active listener/accepted sockets we should kill existing multiplexer and create new one with requested ip:port. this is because existing multiplexer doesn't fit our bind request exactly and it is not used right now. So removing such multiplexer is correct way. We don't care about REUSEADDR value of existing multiplexer is it doesn't have active sockets.
• if multiplexer has active socket such bind attempt must fail regardless of REUSEADDR value for new and existing socket.

2. if you try to bind ip:port and there is multiplexer exists with the same port and requested ip to bind match exactly with ip of this existing multiplexer:

• if this multiplexer exists without any active listener/accepted sockets we should just reuse this existing multiplexer regardless of it's REUSEADDR as there is no any existing listeners anyway. We apply REUSEADDR from this new socket.
• if multiplexer has active listener socket such bind should success if both existing and new sockets are REUSEADDR and should fail otherwise. If success SRT should balance accepted sockets between such listeners.

Actually I collected what is discussed in #1513 (comment) as it contradicts with #1588 (comment)
In particular: in #1513 (comment) some binding attempts should fail regardless of REUSEADDR value while initial task description in #1588 (comment) says that not reusable sockets should bind if existing binding is not reusable as well.
From my POV if we have multiplexer with existing not reusable sockets we should check this binding ip range and fail binding attempt too and we should not try to create new socket.
I think my description is more accurate.

[ethouris]

According to conversation #1513
[...]
Should be changed to:

1. if you try to bind ip:port and there is multiplexer exists with the same port and requested ip to bind intersects but not exactly match with ip of this existing multiplexer:

• if this multiplexer exists without any active listener/accepted sockets

No, this isn't possible. If the number of users of a multiplexer drops to 0 (after every socket that uses it gets closed), the multiplexer is deleted. That's not a case at all.

We start with a situation that we want to do bind with particular IP (including 0 mask) and port. Then we search through existing multiplexers and see if there's any reusable match or conflict.

• if multiplexer has active socket such bind attempt must fail regardless of REUSEADDR value for new and existing socket.

And that's exactly how sharing DOESN'T work :D

If a multiplexer that matches the port number is found, it's being checked if it is in the way at all, and if so, if it is reusable, or conflicting. If you have a multiplexer with a specified IP address and you are trying to bind a different IP address, this multiplexer isn't in the way. In which case you create a new multiplexer.

The multiplexer can be in the way only if:

• new or existing binding specification specifies 0.0.0.0 as IP address
• the found binding to the given port contains the same IP address

If the multiplexer is in the way, it can then only be reusable or conflicting, so after making sure of that we need to check if this multiplexer is reusable, which is if:

• the REUSEADDR flag is true (this value is copied from the first socket that created the multiplexer)
• the UDP specific settings in the multiplexer and the bind-attempted socket are the same
• they have the exactly same IP address specification

Otherwise it's conflicting.

There's also a small confusing situation with IPv6. If an existing binding is for :: address with IP6ONLY=false, then an attempt to bind IPv4 0.0.0.0 address is in conflict with it. Conversely, an attempted bind to IPv6 address :: with IP6ONLY=true is not conflict with (and also isn't in the way of) an existing binding to IPv4 0.0.0.0

From my POV if we have multiplexer with existing not reusable sockets we should check this binding ip range and fail binding attempt too and we should not try to create new socket.
I think my description is more accurate.

But it doesn't match the current (and imaginable) state.

First of all, there's no problem with reusing multiplexers of existing sockets, regardless of their state (unless they are broken and pending deletion, but that's another story). The only thing that is required is that it is allowed to be reused, and all UDP settings and IP address specification are the same.

When you try to bind, there's a search over existing multiplexers (note that this means existing sockets using them). First we look for candidates - which is a multiplexer with the same port, and:

• If attempted binding is for a wildcard, any of them
• if attempted binding is for a specified IP address, only the ones with matching IP address or wildcard
• If attempted binding is IPv6 wildcard with V6ONLY=false (that is, both 4 and 6), any IPv6 and IPv4 wildcard
• If attempted binding is IPv6 wildcard with V6ONLY=true, only IPv6.

This is a selection of "potentially in the way". Then matching is if:

• IPv4 and IP address specification is the same
• IPv6 with both V6ONLY=true and IP address is the same
• IPv6 with both V6ONLY=false and IP address in both specs is a wildcard
• IPV6 with any value of V6ONLY, but both specify the same IP address
• In call cases above, all UDP settings are the same

In all other cases this is a conflicted binding and is rejected.

[alexpokotilo]

According to conversation #1513
[...]
Should be changed to:

1. if you try to bind ip:port and there is multiplexer exists with the same port and requested ip to bind intersects but not exactly match with ip of this existing multiplexer:

• if this multiplexer exists without any active listener/accepted sockets

No, this isn't possible. If the number of users of a multiplexer drops to 0 (after every socket that uses it gets closed), the multiplexer is deleted. That's not a case at all.

Once you call srt_close, socket is market for removal but not removed immediately. It's actually removed from garbage collector after some time. The same time multiplexer removed if there is no socket associated with given multiplexer. I tried to cover the case when given listener socket is closed, but not yet garbage collected, and new socket with the same settings is creating. This is a possible case. I agree that it may be hard to support but it's possible and easy to reproduce for sure as GC works with 1 second delay.

We start with a situation that we want to do bind with particular IP (including 0 mask) and port. Then we search through existing multiplexers and see if there's any reusable match or conflict.

• if multiplexer has active socket such bind attempt must fail regardless of REUSEADDR value for new and existing socket.

And that's exactly how sharing DOESN'T work :D

You probably missed that this condition has following pre-condition in my reply

let me copy this precondition here again:

if you try to bind ip:port and there is multiplexer exists with the same port and requested ip to bind intersects but not exactly match with ip of this existing multiplexer

if I combine explicitly both conditions we will get the following:
"if you try to bind ip:port and there is multiplexer exists with the same port and requested ip to bind intersects but not exactly match with ip of this existing multiplexer" and "if multiplexer has active socket such bind attempt must fail regardless of REUSEADDR value for new and existing socket." .

In your terminology this case is when existing socket is "in the way" and conflicting. so we cannot reuse it regardless of REUSEADDR value and reading your explanation above I see that I'm right and we're talking about the same things.

If a multiplexer that matches the port number is found, it's being checked if it is in the way at all, and if so, if it is reusable, or conflicting. If you have a multiplexer with a specified IP address and you are trying to bind a different IP address, this multiplexer isn't in the way. In which case you create a new multiplexer.

exactly.

The multiplexer can be in the way only if:

* new or existing binding specification specifies 0.0.0.0 as IP address

* the found binding to the given port contains the same IP address

If the multiplexer is in the way, it can then only be reusable or conflicting, so after making sure of that we need to check if this multiplexer is reusable, which is if:

* the REUSEADDR flag is true (this value is copied from the first socket that created the multiplexer)

* the UDP specific settings in the multiplexer and the bind-attempted socket are the same

* they have the exactly same IP address specification

Otherwise it's conflicting.

exactly and this is exactly what I'm talking about in my version of this task description

From my POV if we have multiplexer with existing not reusable sockets we should check this binding ip range and fail binding attempt too and we should not try to create new socket.
I think my description is more accurate.

But it doesn't match the current (and imaginable) state.

please read my explanations above. I admit it may be not very intuitive but it's correct. Let me remove "existing multiplexer without active socket " case from it. here is what we have:

1. if you try to bind ip:port and there is multiplexer exists with the same port and requested ip to bind intersects but not exactly match with ip of this existing multiplexer
   such bind attempt must fail regardless of REUSEADDR value for new and existing socket.
2. if you try to bind ip:port and there is multiplexer exists with the same port and requested ip to bind match exactly with ip of this existing multiplexer,
   such bind should success if both existing and new sockets are REUSEADDR and should fail otherwise. If success SRT should balance accepted sockets between such listeners.

I've read your explanation and it match exactly to these two rules.

But here is what you stated in the task description initially:

You have a binding with the same port.
If so, whether this is a binding that covers the range of the attempted binding
If the found binding is reusable, it may only result in reusage, or in conflict. 
Only if the binding is not reusable does it proceed to create a new binding.

here is how I read fourth statement:
"If found binding intersects with the new one, but existing listener is not reusable we should create new binding." But this is not what we want to do and your explanations on your later reply confirms this.
This last-fourth rule should be something like:
Only if the binding is not "in the way" does it proceed to create a new binding.
If you agree on this, please read my version again. It states the same. It's a little more verbose though.
I don't need you replace your description with mine. I just need to understand we are on the same page.

[ethouris]

According to conversation #1513
[...]
Should be changed to:

1. if you try to bind ip:port and there is multiplexer exists with the same port and requested ip to bind intersects but not exactly match with ip of this existing multiplexer:

• if this multiplexer exists without any active listener/accepted sockets

No, this isn't possible. If the number of users of a multiplexer drops to 0 (after every socket that uses it gets closed), the multiplexer is deleted. That's not a case at all.

Once you call srt_close, socket is market for removal but not removed immediately.

That changes nothing. If the socket isn't yet internally removed, it is still a client o a multiplexer.

I tried to cover the case when given listener socket is closed, but not yet garbage collected, and new socket with the same settings is creating. This is a possible case. I agree that it may be hard to support but it's possible and easy to reproduce for sure as GC works with 1 second delay.

Yes, that could be an issue. I have also once fixed a bug when you close the listener socket and want to create another one with the same port, as this freeing of the listener port was also done only in GC. I fixed that by letting the socket be cleaned up by GC, but the listener occupation of the multiplexer is freed immediately. In general there's no problem at all to bind a caller socket to the same bindspec as an existing listener socket. You only cannot bind a listener socket to a bindspec that is already occupied by a listener socket (that is, you can bind, but after binding you cannot set this socket listen).

The problem could be if you want to set up a listener with bindspec that is in conflict with an existing bindspec that remains after closing a listener socket (e.g.: set up a listener socket bound to 127.0.0.1:5000, close it, set up a new socket bound to 0.0.0.0:5000 and try to listen). This will fail, and could only succeed if you wait about 1 second. Kicking off a multiplexer in conflict would be a considerable idea, but the problem is that not in every case you can forcefully close a multiplexer.

if I combine explicitly both conditions we will get the following:
"if you try to bind ip:port and there is multiplexer exists with the same port and requested ip to bind intersects but not exactly match with ip of this existing multiplexer" and "if multiplexer has active socket such bind attempt must fail regardless of REUSEADDR value for new and existing socket." .

In your terminology this case is when existing socket is "in the way" and conflicting. so we cannot reuse it regardless of REUSEADDR value and reading your explanation above I see that I'm right and we're talking about the same things.

If a multiplexer that matches the port number is found, it's being checked if it is in the way at all, and if so, if it is reusable, or conflicting. If you have a multiplexer with a specified IP address and you are trying to bind a different IP address, this multiplexer isn't in the way. In which case you create a new multiplexer.

Yeah, ok, that's correct. These rules are kinda confusing. :)

1. if you try to bind ip:port and there is multiplexer exists with the same port and requested ip to bind intersects but not exactly match with ip of this existing multiplexer
   such bind attempt must fail regardless of REUSEADDR value for new and existing socket.
2. if you try to bind ip:port and there is multiplexer exists with the same port and requested ip to bind match exactly with ip of this existing multiplexer,
   such bind should success if both existing and new sockets are REUSEADDR and should fail otherwise. If success SRT should balance accepted sockets between such listeners.

I've read your explanation and it match exactly to these two rules.

I still think that "regardless of REUSEADDR" etc. is confusing. In order to be able to bind you have to satisfy ALL of the conditions of case 2 here (with no regard to ordering at all):

1. No intersecting binding found (create a new multiplexer then)
2. An intersecting binding found, then:
   • IP address are identical
   • IPV6ONLY flag is the same
   • REUSEADDR is true
   • UDP-specific settings are identical

But here is what you stated in the task description initially:

You have a binding with the same port.
If so, whether this is a binding that covers the range of the attempted binding
If the found binding is reusable, it may only result in reusage, or in conflict. 
Only if the binding is not reusable does it proceed to create a new binding.

here is how I read fourth statement:
"If found binding intersects with the new one, but existing listener is not reusable we should create new binding." But this is not what we want to do and your explanations on your later reply confirms this.
This last-fourth rule should be something like:
Only if the binding is not "in the way" does it proceed to create a new binding.
If you agree on this, please read my version again. It states the same. It's a little more verbose though.
I don't need you replace your description with mine. I just need to understand we are on the same page.

But then, I think you have contributed well to the description and I think I can make it clearer with this.

[alexpokotilo]

But then, I think you have contributed well to the description and I think I can make it clearer with this.

Made it with great pleasure. I've checked current task description and it's brilliant clear and precise, thank you!

[maxsharabayko]

Unit test is failing in Travis.

ReuseAddr.ProtocolVersion
Bind to: :::5000
Connecting to: ::1:5000
/home/travis/build/Haivision/srt/test/test_reuseaddr.cpp:266: Failure
Expected: (srt_epoll_wait(server_pollid, read, &rlen, write, &wlen, 3000, 0, 0, 0, 0)) != (SRT_ERROR),
actual: -1 vs -1
terminate called without an active exception
/home/travis/.travis/functions: line 109:  5598 Aborted   (core dumped) ./test-srt --gtest_filter="-