0

I'm using Socket.IO like in this sample:

io.sockets.on("connection", function (socket) {

    myService.on("myevent", function() {
        socket.emit("myevent", { /* ... */ });
        // some stuff happens here of course
    });

});

myService is a singleton and a subclass of EventEmitter which triggers the myevent over the time. Anything works fine, however I guess that I create some kind of leak in this case. How does my service know that it doesn't need to call the handler once the connection is destroyed? Is there some kind of destroy event I can catch and then remove the handler from myService?

miho
  • 11,765
  • 7
  • 42
  • 85
  • It is not clear what `myService` is and what it's lifetime is or how it's even related to a socket. `myService` is a singleton what? You can monitor the `disconnect` event and do something there. – jfriend00 Feb 27 '15 at 22:16
  • As mentioned in my question `myService` is a **singleton**. – miho Feb 27 '15 at 22:17
  • A singleton what? What type of object is it? What does it do? What methods does it have? What relation does it have to the socket? A singleton ONLY means that there's only one of them. It doesn't tell us what type of object it is and what it's purpose is and how it's used? You're only disclosing about 1/3 of the relevant issue here. – jfriend00 Feb 27 '15 at 22:18
  • Yes, it does appear you'd have a memory leak here if a socket connects, and then disconnects and the relevant myService event handler is not removed. Your `myService.on()` event handler will retain a closure reference to the `socket` even though it's long since been closed. We'd have to know what myService was and how event handlers can be removed from it to know what to suggest. – jfriend00 Feb 27 '15 at 22:23
  • @jfried00: It's a generalized question anyway, so there is no need to know what it's purpose or which type of object it is. When talking about singletons, I mean an object which is instantiated once and then kept over the whole process lifetime and reused in all places where it is needed. – miho Feb 27 '15 at 22:24
  • Well, then the generalized answer is to listen for each socket disconnect even and remove the myService event handler. Can't be more specific than that with the lack of detail you've provided. – jfriend00 Feb 27 '15 at 22:25
  • Oh, I'm sorry. You are right, I should have mentioned: myService is some subclass of `EventEmitter`. – miho Feb 27 '15 at 22:25

2 Answers2

0

Listen to the socket disconnect event and when you get a disconnect event, remove the relevant event handler from the myService object.

You should be able to do that like this:

io.sockets.on("connection", function (socket) {

    function handler() {
        socket.emit("myevent", { /* ... */ });
        // some stuff happens here of course
    }

    myService.on("myevent", handler);

    socket.on("disconnect", function() {
        myService.removeListener("myevent", handler);
    });

});

If what you're trying to do is to broadcast to all connected sockets, you could just install one "myevent" listener (not one per connection) and use io.emit() to broadcast to all sockets too and not have to handle the connect or disconnect events for this purpose.

jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • Thanks. That's it. The part I missed was the `disconnect` event. However my question wasn't clear, I'm sorry and thanks for your help. – miho Feb 27 '15 at 22:27
  • @miho - I added a code example, now that I know `myService` is an `EventEmitter`. – jfriend00 Feb 27 '15 at 22:29
  • Thank you. Just some small correction, it's [`removeListener`](http://nodejs.org/api/events.html#events_emitter_removelistener_event_listener) not `removeEventListener`. – miho Feb 27 '15 at 23:02
  • It's not a good practice to add/remove listeners to individual sockets when you want to simply broadcast a message to all clients. Check my answer for a much more efficient and cleaner solution. – diego nunes Feb 03 '18 at 22:36
  • @diegonunes - You're making some assumptions about the OP's situation that are not stated in the question. Maybe that's the case, maybe not. They asked how to remove an event listener when the socket disconnects so I showed them how to do that correctly. And, for that, you downvoted me. Geez. If your answer is better it should get more upvotes. You don't need to downvote a correct answer just because you think you have a better way. If the OP supplied more context about the overall problem, we could probably all provide a more useful answer in the grand scheme of things. – jfriend00 Feb 03 '18 at 22:53
  • @jfriend00 I'm assuming based on the code sample. Although he didn't state it, his code is clearly emitting an event to all connected sockets, but in a very inneficient way, adding an event listener at every connection rather than listening once and broadcasting the message. Even if it's just a subset, it's better to use namespacing or even saving the socket list in an array or something. Whatever the objective, adding/removing the same listener at every connection is not a way of doing it. – diego nunes Feb 04 '18 at 15:05
  • @diegonunes - I have no problem with you proposing (in your answer) why your answer is a better overall solution, but downvoting an accepted answer that works just fine 3 years after it was posted and apparently worked just fine for the OP is NOT what you are supposed to use downvotes for. Downvotes are for wrong answers. Upvotes are supposed to distinguish between levels of working answers. If you follow the comment history between myself and the OP, you can see that some info here was not originally disclosed either. – jfriend00 Feb 04 '18 at 15:50
0

If you are planning to send data to all sockets when some other event fires, you don't need to add/remove another listeners every time a client connects/disconnects.

It is a lot more efficient and easier to simply fire the socket.io event to all sockets that are connected right now using io.sockets (which is a reference to the default namespace with all clients on it by default) and io.sockets.emit:

myService.on('myevent', () => {
  io.sockets.emit('myevent', {/*...*/});
});

If you only need to fire this event to some subset of your users, try using specific namespaces or rooms:

myService.on('myevent', () => {

  //with namespaces
  io.of('namespace').emit('myevent', {/*...*/});

  //with rooms
  io.to('room').emit('myevent', {/*...*/});

});
diego nunes
  • 2,750
  • 1
  • 14
  • 16