Opened 16 years ago

Last modified 12 years ago

#1796 enhancement new

Flumotion could use an API-stable and approved way of stealing the underlying socket of a transport from the reactor

Reported by: thomasvs Owned by: thomasvs
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch:
Author:

Description

The use case is the following:

  • use a twisted protocol for initial connection. We have HTTP for a streaming client request coming in, and PB for internal communication between components.
  • after initial protocol handling, steal the underlying file descriptor from the reactor, so that the reactor itself will never read from or write to that socket, and the fd can safely be passed to another chunk of code that will use it
  • bonus points for making it possible to later readd the fd to the reactor somehow and pick up with the original protocol.

Right now, transport.broker.stopReading() and stopWriting() are being used to much the same effect, but exarkun says these are not part of the public API.

He'd prefer a .removeFromReactor() being added instead.

possibly useful IRC snippet:

<exarkun> thomasvs: The use case of extracting a file descriptor completely from the reactor's purview is perhaps a useful one, but not one best satisfied by stopReading and stopWriting.
<thomasvs> actually, it's read and write atm, we're trying to move all of the setup code to twisted, and only deal with fd's
<exarkun> thomasvs: It would be much more useful and convenient to have removeFromReactor(), don't you think?
<thomasvs> PenguinOfDoom: we don't care about win32 atm, though I'm sure we will in the future
<thomasvs> exarkun: yes, that'd be excellent
<PenguinOfDoom> exarkun: How do you convince _socketobject to give up an FD without closing it?
<exarkun> PenguinOfDoom: Irrelevant.
<PenguinOfDoom> exarkun: That was an unrelated question. I'm curious.
<exarkun> thomasvs: So that is an API I would consider adding to Twisted and eventually declaring stable, rather than stopReading and stopWriting.
<MikeS> PenguinOfDoom: you extract the  fd, dup() it, then let it get closed!
<MikeS> For extra credit, avoid having twisted call shutdown() on it
<exarkun> PenguinOfDoom: Basically what MikeS said.
<PenguinOfDoom> Doesn't _socketobject call shutdown itself?
<thomasvs> exarkun: so, what do I do ? file a ticket ? try implementing it ?
<MikeS> PenguinOfDoom: not by itself
<PenguinOfDoom> Oh, right, it does not.
<PenguinOfDoom> haha stupid socket, we have a way to defeat you
<exarkun> thomasvs: Those are both excellent things to do :)  Although I would suggest continuing with your current strategy for the time being as well. ;)
<thomasvs> exarkun: yep, but I'd like to push something we can rely on in the future as well
<thomasvs> lemme start with the ticket and assign it to me
<MikeS> PenguinOfDoom: I went through this recently, when I needed to defeat sockets.
<PenguinOfDoom> MikeS: I want a mummified head of an AF_INET socket for a wall decoration.
<thomasvs> exarkun: what would removeFromReactor remove ? the transport ? the actual fd ?
<exarkun> thomasvs: It would depend on how the reactor was implemented.
<PenguinOfDoom> thomasvs: Will you want to give it back to the reactor in the future?
<thomasvs> PenguinOfDoom: me, no.  maybe other users of this API
<PenguinOfDoom> So it's not really removeFromReactor. It's stopDoingThingsWithThisSocket. And takeItBackPlease
<idnar> addToReactor
<PenguinOfDoom> idnar: no such thing

Change History (7)

comment:1 Changed 16 years ago by Glyph

Owner: changed from Glyph to Jean-Paul Calderone

Since he was the guy talking on IRC

comment:2 Changed 13 years ago by Jean-Paul Calderone

Resolution: invalid
Status: newclosed

I'm not sure why I thought IReactorFDSet didn't satisfy this requirement. Perhaps I didn't know about it at the time, or didn't understand it properly. Another possibility is that I thought there should be an API which is the same across the IReactorFDSet reactors and IOCP reactor. However, I do think IReactorFDSet is the proper resolution to this ticket now. It allows you to remove and later re-add readers and writers, it's public, it's stable.

I'm selecting the "invalid" resolution because IReactorFDSet was always there, so this ticket was already resolved when it was filed.

comment:3 Changed 13 years ago by thomasvs

Ok. For the record, we're actually now using reactor.removeReader, which I assume is similarly public API and ok to use.

I've added a comment linking to this bug to ensure we have a paper trail.

Thanks for clarifying!

comment:4 Changed 13 years ago by Jean-Paul Calderone

Yep, removeReader is one of the methods defined by IReactorFDSet.

comment:5 Changed 12 years ago by thomasvs

Resolution: invalid
Status: closedreopened

comment:6 Changed 12 years ago by thomasvs

Owner: changed from Jean-Paul Calderone to thomasvs
Status: reopenednew

I should add a comment saying that Twisted will not itself call protocol.connectionLost if you use this method. Arguably this is a bug, in which case it needs to be fixed in a backwards-compatible way.

comment:7 Changed 12 years ago by Glyph

Why is this a bug? The connection was not, in fact, 'lost' in the sense that it was disconnected; by doing stopReading and stopWriting, you're asking Twisted to stop notifying you about that kind of thing. There's no longer a reference to the IProtocol from the reactor anywhere, so it should just be garbage collected.

If the code which removes the FD from the reactor can call connectionLost if it wants to.

Note: See TracTickets for help on using tickets.