Raise exception when port given to server is already occupied

[adamkpickering]

I am trying to implement an iperf3 server that is capable of doing multiple simultaneous tests. I am planning on doing the handling of multiple tests using Twisted. Unfortunately I've run into an issue with finding a port to give the iperf3.Server: if I give it a port that is already being used by another process, no error information is returned until the call to Server.run() returns. Until this point there is no way of telling if the port was good or not.

It seems to me that the pythonic way to do this would be to define an exception ("PortTakenError" or something similar) that is raised before the call to Server.run() returns. That way, people can encapsulate the Server.run() call in a try-except statement and handle a "PortTakenError" gracefully.

Is this doable? I can probably figure out how to implement this; if I submit a pull request with these changes would you accept it?

[thiezn]

Hi @adamkpickering,

The problem with that solution is that when you instantiate a iperf3.Server object, the port number is just stored in the port variable. It doesn't reserve any port on the network stack of the machine. The actual server port will only be opened when starting the server (server.run()).

Building in a check during instantiation will not provide any guarantee that the port isn't taken when you call the Server.run() function. Let's me try to illustrate:

Say you have built in a check for this when you set the server port variable :

server = iperf.Server()

# Here we set the port and lets say the module checks and sees no process is running on port 1234
server.port = 1234

# Now we do some more stuff in the script before actually starting the server
# but during this process some other process on the machine starts a server
# on port 1234
sleep(10)

# You finished your 10 seconds of sleep, think all is well and run the server...
#  Unfortunately the port is now already taken and you still get
# an error.
server.run()

I don't have a machine available at the moment to test but to solve your issue I would suggest to do something like this:

server = iperf.Server()
server.port = 1234

try:
   server.run()
except NotSureWhatExceptionGetsRaisedByDefaultForPortInuse:
  server.port += 1
  server.run()

Cheers,

[adamkpickering]

I don't have a machine available at the moment to test but to solve your issue I would suggest to do something like this:

server = iperf.Server()
server.port = 1234

try:
   server.run()
except NotSureWhatExceptionGetsRaisedByDefaultForPortInuse:
  server.port += 1
  server.run()

The problem with doing that is that server.run() does not raise exceptions (beyond the built-in ones, and definitely not for an occupied port), so the except statement does not get called no matter what. Instead, the call to server.run() returns good JSON for a successful test or the JSON "error": "unable to start listener for connections: " in the event that the specified port was occupied. So one way to solve the problem would be:

server = iperf.Server()
server.port = 1234
server.run()
# wait about 2 seconds (that is the time it takes for server.run() to return with an
# error message in the event of an error)
# if server.run() has not yet returned assume everything went fine
# subsequent code here

But this is a hack. It would require another thread to monitor server.run(), and who wants that? And what if the amount of time it takes for server.run() to return with an error changes? There really are no nice solutions as it is. I know that it is a big ask, but I think that a small change in code function is needed. There needs to be some way to tell, without any doubt, whether the server was able to start successfully. I can think of two ways of doing this:

1. Introduce state to the Server object. The Server object would have, for example, a boolean started property that was initially equal to False. A call to server.run() would attempt to start the server, and if there were no errors the call would change server.started to True. If an error occurred with starting the server a suitable exception could be raised.

2. Have server.run() return another object that represents the running server, rather than having a started property. IMO this is worse because it is a bigger change.

In both cases we would need a new way to get data off the object from tests. A generator would be nice here (for getting data as it comes in during a test). Even a plain old property could hold the final results. Again, I realize this is a big ask with breaking changes so if it doesn't sound good I can just fork it and work on that.

[thiezn]

Hi @adamkpickering

I see what you mean, there is indeed no exception raised but rather an error provided in the TestResult. I've quickly created a new branch https://github.com/thiezn/iperf3-python/commit/7ae5a6a0deac97d0935b8398b75d09dc3f0ea74a to see if it makes sense to raise an exception when an error occurs. Have a look at the branch to see if this is something usable for you. I would have to see if there's a sensible standard exception or create a new one.

Here's the output you get when a port is already taken:

>>> import iperf3
>>> server = iperf3.Server()
>>> server.port = 3306
>>> server.run()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "development/iperf3-python/iperf3/iperf3.py", line 680, in run
    return TestResult(data_queue.get())
  File "development/iperf3-python/iperf3/iperf3.py", line 767, in __init__
    raise IOError(self.json['error'])
OSError: unable to start listener for connections:

[adamkpickering]

I think that is better than it currently is, and that it is worth implementing. However, after digging into the iperf3 library a bit I've discovered that the problem is with the library (not to say it's bad, just that it is not made to play nicely with Python), not your wrapper. There is nothing that can be done to make iperf3-python work as I described above short of modifying the iperf3 library itself and then making corresponding changes in iperf3-python. And that would be prohibitively time-consuming.

But I agree that what you did above is an improvement. I'll pursue other paths to solve my problem :)