HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Multi-threaded socket client for Scrolls

Submitted by: @import:stackexchange-codereview··
0
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

Solution

Instead of:

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 = True


That 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
            pass

Code Snippets

Thread.__init__(self)
super(MessageThread, self).__init__()
while True:
self.active = True
while self.active:

Context

StackExchange Code Review Q#27909, answer score: 2

Revisions (0)

No revisions yet.