patternpythonMinor
Multi-threaded socket client for Scrolls
Viewed 0 times
multiclientforsocketthreadedscrolls
Problem
This is a multi-threaded socket client written to talk to the Scrolls socket server. The idea is to send commands to the socket server and respond to messages received via callbacks. I've never done multi-threading before and would like the code reviewed for code correctness, best practices, and potential issues regarding how I'm handling threading.
GitHub
```
from Crypto.Cipher import PKCS1_v1_5
from Crypto.PublicKey import RSA
from base64 import b64encode
from threading import Thread
from Queue import Queue
import socket
import json
import time
class PingThread(Thread):
def __init__(self, scrolls_client):
self.scrolls_client = scrolls_client
self.stopped = False
Thread.__init__(self)
def run(self):
while not self.stopped:
self.scrolls_client.send({'msg': 'Ping'})
time.sleep(10)
class MessageThread(Thread):
def __init__(self, scrolls_client):
self.scrolls_client = scrolls_client
self.stopped = False
Thread.__init__(self)
def run(self):
while not self.stopped:
# grab a message from queue
message = self.scrolls_client.queue.get()
# make a copy of the current subscribers to keep this thread-safe
current_subscribers = dict(self.scrolls_client.subscribers)
# send message to subscribers
for subscriber_key, subscriber_callback in current_subscribers.iteritems():
# msg or op should match what we asked for
if 'msg' in message and message['msg'] == subscriber_key:
subscriber_callback(message)
elif 'op' in message and message['op'] == subscriber_key:
subscriber_callback(message)
# signals to queue job is done
self.scrolls_client.queue.task_done()
class ReceiveThread(Thread):
def __init__(self, scrolls_client):
self.scrolls_client = scrolls_client
self.stopped = Fa
GitHub
```
from Crypto.Cipher import PKCS1_v1_5
from Crypto.PublicKey import RSA
from base64 import b64encode
from threading import Thread
from Queue import Queue
import socket
import json
import time
class PingThread(Thread):
def __init__(self, scrolls_client):
self.scrolls_client = scrolls_client
self.stopped = False
Thread.__init__(self)
def run(self):
while not self.stopped:
self.scrolls_client.send({'msg': 'Ping'})
time.sleep(10)
class MessageThread(Thread):
def __init__(self, scrolls_client):
self.scrolls_client = scrolls_client
self.stopped = False
Thread.__init__(self)
def run(self):
while not self.stopped:
# grab a message from queue
message = self.scrolls_client.queue.get()
# make a copy of the current subscribers to keep this thread-safe
current_subscribers = dict(self.scrolls_client.subscribers)
# send message to subscribers
for subscriber_key, subscriber_callback in current_subscribers.iteritems():
# msg or op should match what we asked for
if 'msg' in message and message['msg'] == subscriber_key:
subscriber_callback(message)
elif 'op' in message and message['op'] == subscriber_key:
subscriber_callback(message)
# signals to queue job is done
self.scrolls_client.queue.task_done()
class ReceiveThread(Thread):
def __init__(self, scrolls_client):
self.scrolls_client = scrolls_client
self.stopped = Fa
Solution
Instead of:
Do (for example):
Also it's more Pythonic to do:
Instead of:
Instead of using the stopped variables, it may be more efficient to use a variable such as:
That way you can do this which saves you from having to do extra processing on the "not" every iteration:
Even better though may be to just do while True and instead of setting stopped to True and calling Thread_stop(), just call exit() on the threads, then you shouldn't need the stopped variables at all.
Also in receive() you could simplify it slightly
Thread.__init__(self)Do (for example):
super(MessageThread, self).__init__()Also it's more Pythonic to do:
while True:Instead of:
while (1):Instead of using the stopped variables, it may be more efficient to use a variable such as:
self.active = TrueThat way you can do this which saves you from having to do extra processing on the "not" every iteration:
while self.active:Even better though may be to just do while True and instead of setting stopped to True and calling Thread_stop(), just call exit() on the threads, then you shouldn't need the stopped variables at all.
Also in receive() you could simplify it slightly
def receive(self):
stream_data = ''
while True:
# read data from the buffer
data = self.socket.recv(self._socket_recv)
if not data:
# no more data being transmitted
return # now you don't need an else
# append data to the response
stream_data += data
try:
# line breaks means we are handling multiple responses
if stream_data.find("\n\n"):
# split and parse each response
for stream_data_line in stream_data.split("\n\n"):
# we have a response, add it to the queue
# no need to store json.loads result that's only used once
self.queue.put(json.loads(stream_data_line))
except:
# invalid json, incomplete data
passCode Snippets
Thread.__init__(self)super(MessageThread, self).__init__()while True:self.active = Truewhile self.active:Context
StackExchange Code Review Q#27909, answer score: 2
Revisions (0)
No revisions yet.