The below code is a simple TCP echo server. The folks using it claim "it is going down every now and again". Any thoughts on how this could be improved would be really appreciated, particularly if they would improve the reliability.
import select, argparse, socket
def server_socket(ip, port):
serv = socket.socket()
try:
serv.bind((ip, port))
except:
if (ip != ''):
print("Failed to bind to " + ip + " on port " + str(port) + " exiting" )
else:
print("Failed to bind to all on port " + str(port) + " exiting" )
exit()
if (ip != ''):
print("Bound to " + ip + " on port " + str(port) + " echoing tcp" )
else:
print("Bound to all on port " + str(port) + " echoing tcp" )
serv.listen(1)
inputlist = [serv,]
while 1:
inputready,_,_ = select.select(inputlist,[],[], 20)
if not inputready:
for i in inputlist:
if i != serv:
i.close()
inputlist.remove(i)
print("connection timeout")
for i in inputready:
if i == serv:
client, address = serv.accept()
inputlist.append(client)
print("connection open from " + str(address))
else:
try:
data = i.recv(1024)
print("received " + str(len(data)) + " bytes"),
print(" \""+ data +"\"")
if data:
i.send(data)
else:
i.close()
inputlist.remove(i)
print("connection closed")
except:
i.close()
inputlist.remove(i)
print("connection closed")
if __name__ == '__main__':
parser = argparse.ArgumentParser(prog = "TCP Echo server")
parser.add_argument('-i', '--ip', type=str, default='', help = "IP to bind to")
parser.add_argument('port', type=int, help = "Port to bind to")
args = parser.parse_args()
server_socket(args.ip, args.port)
1 Answer 1
The folks using it claim "it is going down evey now and again".
Well, yes. If they don't send you a message in 20 seconds, you're disconnecting them! Why? Simply removing that block is probably what you want.
Separation of Concerns
Your server_socket
function both creates a server and handles the busy loop. You should split those into two functions:
def make_server(ip, port):
...
def run_echo(server):
...
server = make_server(ip, port)
run_echo(server)
Echoing
You wrap your echo block in a try
/except
. Why? What part of that code can raise an exception? What exception is raised? You should prefer to only catch precisely the exceptions that are raised (to avoid, for instance, catching SyntaxError
and hiding whatever typo you wrote in your code), but in this case I don't know what exception you would be catching.
Also, you don't need to print str(len(data))
, you can just print len(data)
.
-
\$\begingroup\$ Thanks very much for your thoughts, Just to be clear, "going down" means no new connections can be made. The 20 second cutoff is by design, If the connection to a client is lost, will the socket remain open indefinitely without this block? You also mentioned the try/catch around the echo portion, I believe
recv
throws an exception when there is no data to be read, you are quite correct in that I should specify this particular exception. \$\endgroup\$jayjay– jayjay2015年10月23日 17:14:43 +00:00Commented Oct 23, 2015 at 17:14