Opened 11 years ago

Last modified 7 years ago

#365 defect new

Reactor should track associated sockets separately from read/write state.

Reported by: etrepum Owned by:
Priority: high Milestone:
Component: core Keywords:
Cc: glyph, radix, spiv, itamarst, etrepum, warner, jknight, therve Branch:
Author: Launchpad Bug:

Description


Change History (13)

comment:1 Changed 11 years ago by etrepum

Reactor methods such as removeAll() and disconnectAll() assume that all sockets are currently 
reading.  This is not the case.

The connection phase for a socket should not remove it entirely from the reactor and put it back.  
Something else needs to happen here, so a wrapper can be preserved (as in cfreactor).  There's 
currently a nasty hack in cfreactor to workaround this, the hack should go away.

Sockets need to tell the reactor when they lose connection.

Support for half-duplex sockets (shutdown) should happen.

comment:2 Changed 11 years ago by radix

why is this urgent? do you expect this to be in 1.1.0?

comment:3 Changed 11 years ago by etrepum

ok, I guess it's ok to release 1.1 without these fixes, considering twisted has had these bugs 
forever.. but they're pretty gnarly and should go away ASAP.

comment:4 Changed 11 years ago by moshez

I'm downgrading this to feature.
This should not, of course, deter anyone from working on it!

comment:5 Changed 11 years ago by etrepum

The first part is most certainly a bug, it's causing warner problems because the implementation of 
removeAll() leaves dead write sockets on the reactor.  I believe this probably happens when a 
consumer is paused, and the socket is removed. 

The second part is just a whack implementation that should be fixed.  

I'd consider the third part a bug as well (IIRC, if you manually do loseConnection on a transport, the 
reactor doesn't find out about it until some error happens on the file descriptor).  

The fourth, is a feature.

comment:6 Changed 10 years ago by spiv

The fourth (half-duplex sockets) already has a seperate issue: #223.

comment:7 Changed 10 years ago by hypatia

Assigning to itamar

comment:8 follow-up: Changed 9 years ago by jknight

The only issue remaining out of the original bunch is that reactors should track 
sockets they are associated with, not just sockets that are currently reading or 
writing. 

This is desirable for a bunch of non-select reactors. CFReactor wants to make 
wrappers, pollreactor can notice connection errors while not reading/writing, 
it'd be good for the future epollreactor to not have to remove/add from the 
watched set as often, and maybe more.

comment:9 in reply to: ↑ 8 Changed 7 years ago by therve

  • Cc therve added

Replying to jknight:

pollreactor can notice connection errors while not reading/writing

Can you elaborate on this part? I've tried to detect connection lost with a socket only registered for POLLERR and POLLHUP (which seems to be ignored), but didn't get the event. It seems I can't get notification if not registered for POLLIN or POLLOUT.

comment:10 Changed 7 years ago by jknight

import select
p=select.poll()
import socket
s=socket.socket()
s.bind((, 1234))
s.listen(10)
cs,ca=s.accept()
p.register(cs.fileno(), 0)
p.poll()


import socket, struct
s=socket.socket()
s.connect(('localhost', 1234))
s.send('hi')
s.setsockopt(socket.SOL_SOCKET, socket.SO_LINGER,

struct.pack("ii", 1, 0))

s.close()

comment:11 Changed 7 years ago by jknight

Erf, sorry for that formatting, let's try again...

import select
p=select.poll()
import socket
s=socket.socket()
s.bind(('', 1234))
s.listen(10)
cs,ca=s.accept()
p.register(cs.fileno(), 0)
p.poll()

----
import socket, struct
s=socket.socket()
s.connect(('localhost', 1234))
s.send('hi')
s.setsockopt(socket.SOL_SOCKET, socket.SO_LINGER,
  struct.pack("ii", 1, 0))
s.close()

comment:12 Changed 7 years ago by therve

Thanks for the insight, I completely forgot SO_LINGER. I have also made tests with SO_KEEPALIVE, which also get a notification, but later on.

But if I understand correctly, SO_LINGER should be set on the remote socket for the local socket to be notified. If I have a server which get random connections, I can't expect client to set this correctly...

To be clear, it seems a server can't stop reading/writing to a socket and expect to be reliably notified of socket closing.

comment:13 Changed 3 years ago by <automation>

  • Owner jknight deleted
Note: See TracTickets for help on using tickets.