Code Monkey home page Code Monkey logo

Comments (8)

kdschlosser avatar kdschlosser commented on July 16, 2024 1

all code blocks are pseudo code. They have not been tested specifically in the websocket_server library

OK so this an idea that works in a test setup of simple class instances and it should as well in the websocket_server lib.

class API():

    def run_forever(self):
        try:
            logger.info("Listening on port %d for clients.." % self.port)
            self.serve_forever()
        except KeyboardInterrupt:
            self.server_close()
            logger.info("Server terminated.")
        except Exception as e:
            logger.error(str(e), exc_info=True)
            exit(1)

    def new_client(self, client, server):
        pass

    def client_left(self, client, server):
        pass

    def message_received(self, client, server, message):
        pass

    def set_fn_new_client(self, fn):
        self.new_client = fn

    def set_fn_client_left(self, fn):
        self.client_left = fn

    def set_fn_message_received(self, fn):
        self.message_received = fn

    def send_message(self, client, msg):
        self._unicast_(client, msg)

    @classmethod
    def send_message_to_all(cls, msg):
        self._multicast_(msg)


class WebsocketServer(ThreadingMixIn, TCPServer, API):
    allow_reuse_address = True
    daemon_threads = True  # comment to keep threads alive until finished

    clients = []
    id_counter = 0

    def __init__(self, port, host='127.0.0.1', loglevel=logging.WARNING):
        self.send_message_to_all = self.__send_message_to_all
        logger.setLevel(loglevel)
        TCPServer.__init__(self, (host, port), WebSocketHandler)
        self.port = self.socket.getsockname()[1]

    def __send_message_to_all(self, msg):
        for client in self.clients:
            if client['server'] == self:
                self._unicast_(client, msg)

    def _message_received_(self, handler, msg):
        self.message_received(self.handler_to_client(handler), self, msg)

    def _ping_received_(self, handler, msg):
        handler.send_pong(msg)

    def _pong_received_(self, handler, msg):
        pass

    def _new_client_(self, handler):
        self.id_counter += 1
        client = {
            'id': self.id_counter,
            'handler': handler,
            'address': handler.client_address,
            'server': self
        }
        self.clients.append(client)
        self.new_client(client, self)

    def _client_left_(self, handler):
        client = self.handler_to_client(handler)
        self.client_left(client, self)
        if client in self.clients:
            self.clients.remove(client)

    def _unicast_(self, to_client, msg):
        to_client['handler'].send_message(msg)

    def _multicast_(self, msg):
        for client in self.clients:
            self._unicast_(client, msg)

    def handler_to_client(self, handler):
        for client in self.clients:
            if client['handler'] == handler:
                return client

Now what i did here was I had the instance override the class method of the parent class. This way it can be used both ways.

from websocket_server import WebsocketServer

instance1 = WebsocketServer(1234)
instance2 = WebsocketServer(4321)

instance1.send_message_to_all('message sent to only instance1 clients')
WebsocketServer.send_message_to_all('message sent to all clients on all websocket servers')

I think this would be the most elegant solution. and the one that makes the most sense.

being able to access one instance of a websocket server from another instance I do not believe is the best idea. You may even consider changing the clients container to have a mangled name. Then add a property that will iterate the mangled name container and only return clients that pertain to it.

In the original code there was no way to tell what client belong to what websocket connection even if you iterated over the devices container. No marker that was easily had. Without really digging into it but I think that client['handler'].server might point back to the websocket instance, but I am not sure. Event if you do not make clients a mangled name at least make it a private one.

This adds the mangled name which offers protection against inadvertently sending data to the wrong websocket connection if accessed from a websocket instance.

class WebsocketServer(ThreadingMixIn, TCPServer, API):
    allow_reuse_address = True
    daemon_threads = True  # comment to keep threads alive until finished

    __clients = []
    id_counter = 0

    def __init__(self, port, host='127.0.0.1', loglevel=logging.WARNING):
        self.send_message_to_all = self.__send_message_to_all
        logger.setLevel(loglevel)
        TCPServer.__init__(self, (host, port), WebSocketHandler)
        self.port = self.socket.getsockname()[1]
    
    @property
    def clients(self):
        return list(
            client for client in self.__clients
            if client['server'] == self
        )    

    def __send_message_to_all(self, msg):
        for client in self.clients:
            self._unicast_(client, msg)

    def _message_received_(self, handler, msg):
        self.message_received(self.handler_to_client(handler), self, msg)

    def _ping_received_(self, handler, msg):
        handler.send_pong(msg)

    def _pong_received_(self, handler, msg):
        pass

    def _new_client_(self, handler):
        self.id_counter += 1
        client = {
            'id': self.id_counter,
            'handler': handler,
            'address': handler.client_address,
            'server': self
        }
        self.__clients.append(client)
        self.new_client(client, self)

    def _client_left_(self, handler):
        client = self.handler_to_client(handler)
        self.client_left(client, self)
        if client in self.__clients:
            self.__clients.remove(client)

    def _unicast_(self, to_client, msg):
        to_client['handler'].send_message(msg)

    def _multicast_(self, msg):
        for client in self.__clients:
            self._unicast_(client, msg)

    def handler_to_client(self, handler):
        for client in self.__clients:
            if client['handler'] == handler:
                return client

from python-websocket-server.

kdschlosser avatar kdschlosser commented on July 16, 2024

I just stumbled across this as well. the declaration of the client storage needs to be moved from the class level into the constructor.
I personally agree with @PierreRust that this should be fixed.

@PierreRust, I think you should do a PR for it, Took me a while to find the issue. I thought it was something in my code (20K + lines). I kept on thinking I was making a mistake somewhere. 2 days of hunting and I finally checked the library, took me a while to see it. I wasn't looking for that to be the issue.

This is a great lib. does the job and isn't bloated.

from python-websocket-server.

kdschlosser avatar kdschlosser commented on July 16, 2024

I did want to post a simple fix in case anyone else happens across this issue. This should do the trick. put this into a file in your script and import it before you import the websocket_server library. this should monkey patch the library to work properly.

import sys
import websocket_server
from websocket_server import WebsocketServer

del WebsocketServer.clients

_old_websocket_server = WebsocketServer

class WebSocket(WebsocketServer):
    __doc__ = _old_websocket_server.__doc__

    def __init__(
        self,
        port,
        host='127.0.0.1',
        loglevel=websocket_server.logging.WARNING
    ):
        self.clients = []
        WebsocketServer.__init__(self, port, host, loglevel)


sys.modules['websocket_server'].WebsocketServer = WebSocket

from python-websocket-server.

geekbozu avatar geekbozu commented on July 16, 2024

Do we want this to be changed to having a send_all_global and a send_all_handler? This is a glaring issue that has caused me and some friends quite a few bugs and I'm more then willing to implement a fix once we have a proper direction.

from python-websocket-server.

kdschlosser avatar kdschlosser commented on July 16, 2024

you could have a class method something like send_all_sockets and a method that is send_all_clients. have a class level list for all of the socket instances. and the class method would simply iterate over the sockets and call the send_all_clients.

the issue is the class level clients container. every client connection made gets put into it for every websocket instance made. so if you call the send_all when it iterates over the clients container it is sending to clients that are not apart of the websocket instance. ya end up having data go where ya may not want it.

I am not really 100% sure what the reasoning would be to use a specific websocket instance and be able to access the clients from a completely different websocket instance. But there may be a need by some. Hence the reason for making a class method and also an instance method.

I want to tinker with an idea that may satisfy both needs.

from python-websocket-server.

geekbozu avatar geekbozu commented on July 16, 2024

I mean the simple and (IMO) correct solution is to just move the clients declaration inside init.

    class WebsocketServer(ThreadingMixIn, TCPServer, API):
  
        allow_reuse_address = True
        daemon_threads = True  # comment to keep threads alive until finished
        id_counter = 0

        def __init__(self, port, host='127.0.0.1', loglevel=logging.WARNING):
           self.clients = []
           logger.setLevel(loglevel)
           TCPServer.__init__(self, (host, port), WebSocketHandler)

This makes it instance specific which makes it act as one would expect by default, In the vain of keeping backwards compatibility however....

from python-websocket-server.

kdschlosser avatar kdschlosser commented on July 16, 2024

Yeah it is. But i do not know if the placement of clients was intentional.

This has been an issue for a long while and I do not know if the original author is still supporting it or not. So we do not know the original design and purpose.

there are a bunch of ????'s on that portion of it.

I did submit a PR for the modified version of the code above. There are errors in the code above. so it was for example purposes as stated.

from python-websocket-server.

Pithikos avatar Pithikos commented on July 16, 2024

So the original design simply dealt with a single instance. But all this makes sense so v0.5.1 should have this fixed.

from python-websocket-server.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.