Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closing a server does not disconnect clients #495

Closed
josephg opened this issue Aug 30, 2011 · 8 comments
Closed

Closing a server does not disconnect clients #495

josephg opened this issue Aug 30, 2011 · 8 comments
Labels
bug Something isn't working

Comments

@josephg
Copy link

josephg commented Aug 30, 2011

This server calls s.server.close() 1 second after a client connects.

io = require 'socket.io'

s = io.listen 4321

s.sockets.on 'connection', (socket) ->
    setTimeout (-> s.server.close()), 1000

... however, the client is never disconnected:

$ coffee server.coffee 
   info  - socket.io started
   debug - client authorized
   info  - handshake authorized 4096240811453695407
   debug - setting request GET /socket.io/1/websocket/4096240811453695407
   debug - set heartbeat interval for client 4096240811453695407
   debug - client authorized for 
   debug - websocket writing 1::
new connection
client connected. Server will close in 1 second...
   debug - emitting heartbeat for client 4096240811453695407
   debug - websocket writing 2::
   debug - set heartbeat timeout for client 4096240811453695407
   debug - websocket received data packet 2::
   debug - got heartbeat packet
   debug - cleared heartbeat timeout for client 4096240811453695407
   debug - set heartbeat interval for client 4096240811453695407
   debug - emitting heartbeat for client 4096240811453695407
   debug - websocket writing 2::
   debug - set heartbeat timeout for client 4096240811453695407
   debug - websocket received data packet 2::
   debug - got heartbeat packet
   debug - cleared heartbeat timeout for client 4096240811453695407
   debug - set heartbeat interval for client 4096240811453695407
   debug - emitting heartbeat for client 4096240811453695407
   debug - websocket writing 2::

....etc
@3rd-Eden
Copy link
Contributor

3rd-Eden commented Sep 7, 2011

Isn't that Node.js core issue instead of a Socket.IO issue? I would say that Node should clean up all fd's when you want to close the server.

@josephg
Copy link
Author

josephg commented Sep 7, 2011

Quite possibly, especially considering the server -still doesn't quit even after the client has disconnected- (Actually, it does quit, exactly 15 seconds later). If the listening socket were closed but the client socket wasn't closed, you'd expect closing the client socket to terminate the process at last.

@josephg
Copy link
Author

josephg commented Sep 7, 2011

I just played with it -

Given this server:

server = require('http').createServer (req, res) ->
    res.writeHead 200, 'Content-Type': 'text/plain'
    res.write 'Hello world\n'

    setTimeout (-> console.log 'closing'; server.close()), 1000

server.listen 4321

lazy client:

% curl localhost:4321

Just like with the socket.io code:

  • server.close() causes the server's listening socket to be closed immediately
  • The app doesn't close at this point (closing the server socket does not close the client socket).

With the native node.js app, the server terminates as soon as the last client connection is closed. With socket.io, the server still closes on its own, but it waits a full 15 seconds after the last client manually disconnects and the process terminates.

I think node.js is probably acting correctly according to its spec. It sort of makes sense to deal with the last http requests properly while a server is shutting down. .. But I don't think the behaviour is the right one in socket.io. (Terminating all the open connections is certainly what I expected to happen when I called server.close()).

@3rd-Eden
Copy link
Contributor

Yes and no, we can just loop over all clients and send a disconnect() packet. So I'm marking this as bug.

@setthase
Copy link

Does somebody look at this after so long time? Is this on roadmap for one of next release?

@jtmalinowski
Copy link

dang, I just hit this issue when writing tests

@samsonradu
Copy link

@JakubMal I'm interested in seeing the test. Could you please post it?

@jtmalinowski
Copy link

@samsonradu sorry man, my bad! Should have checked twice...

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants