Re: [Twisted-Python] Implementing Postfix Inet Policy Check Client
On Wed, Nov 18, 2015 at 9:28 AM, Tom Boland wrote: > I think what you've provided me with is useful for me, but I think it's > backwards for my purposes, as I need to be connecting to the policy daemon > rather than being the policy daemon! > > I wanted to do this with deferred calls in case one of the policy daemons > becomes unreachable and blocks my application. Do you think I should do > something differently in that regard? My SQL lookups are done > synchronously. If the database server goes away, I've got bigger problems > anyway! > So maybe something like this is more likely to be useful: #!/usr/bin/env python from __future__ import print_function from twisted.internet import reactor, protocol, endpoints, defer from twisted.protocols import basic class PostfixProtocol(basic.LineReceiver): # Assuming Postfix uses '\r\n' line breaks (does it?) delimiter = '\r\n' def __init__(self): self.action = None self.action_deferred = None def lineReceived(self, line): if '=' in line: self.action = line.split('=')[1] elif line == '': self.action_deferred.callback(self.action) self.action_deferred = None else: # oops, bad input pass def sendPostfixRequest(self, request_dict): if not self.action_deferred is None: raise Exception('transaction pending') for k, v in request_dict.items(): self.sendLine('{}={}'.format(k,v)) # Empty line signals we're done self.sendLine('') self.action_deferred = defer.Deferred() return self.action_deferred @defer.inlineCallbacks def checkPostfixPolicy(request_dict): ep = endpoints.clientFromString(reactor, 'tcp:host=127.0.0.1: port=1') p = yield endpoints.connectProtocol(ep, PostfixProtocol()) action = yield p.sendPostfixRequest(request_dict) print('got: {}'.format(action)) reactor.stop() if __name__ == '__main__': request_dict = { 'recipient': 'em...@ddr.ess', 'sender': 'em...@ddr.ess', } reactor.callWhenRunning(checkPostfixPolicy, request_dict) reactor.run() Highlights: - This is not the same protocol as before, in particular it uses a different delimiter. - It assumes the response is also terminated with an empty line (does it?). - It more than one outstanding response: a different exception should be used. - The input processing is very rudimentary and failure-prone. - checkPostfixPolicy could, of course, return instead of printing. :) Cheers, -- exvito ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] Implementing Postfix Inet Policy Check Client
On Thu, Nov 19, 2015 at 4:19 PM, Tom Boland wrote: > Thanks again for this. It's really useful. It turns out that the > delimiter is a plain old \n. Who knows how consistent this will be between > different policy daemons, I don't know! > I would check the Postfix docs for that. > Now, this isn't a working example, it's just the minimum that will > demonstrate my idea. I just wonder if what I've done with the > DeferredQueue is sane. If I return the .get() entry from the DeferredQueue > when doing the request, and then do a put() in lineReceived, am I > guaranteeing that I will get my results in the correct order? > The DeferredQueue is a nice approach: it ensures the get() results come out in the same order as the put() calls. The key question is whether or not the server handles multiple outstanding requests within the same connection. Given your informal protocol description, if the server supports it, it seems the responses must come back in the same order as the requests were sent, otherwise there is apparently no way to relate them; that being the case, what's the advantage of pushing more than one request at a time if one slow request/response "transaction" will delay a subsequent fast request/response "transaction"? A variation of this, assuming the server only handles one outstanding request at a time per connection, could be a protocol implementation that would queue requests ensuring only one was sent at a time: this might provide a cleaner client side API. PS: My references to "Postfix docs" might need to be replaced by "your policy server docs" which, hopefully, will match Postfix's... (you would know that) :) Cheers, -- exvito ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
[Twisted-Python] Windows spawnProcess - Child process inherits files and sockets (POSIX does not)
Hello all, I filed https://twistedmatrix.com/trac/ticket/7970 last summer while focused on a particular project that was hit by it. Since then I had to attend to other things and I'm now back to refocusing on that project. The subject says most of it, but the ticket has more information, including why this isn't an issue on Python >= 3.4 ( my project is on 2.7...) :/ Before submitting a patch for review, I'm looking for preliminary feedback, assuming you agree that the Windows vs POSIX semantics should be the same (if not, why?). My patch calls a few Windows APIs via ctypes, however, as far as I can tell, Twisted on Windows requires pywin32 and, recently, there has been some discussion around dropping that dependency and moving towards something based on cffi. What would you say the way forward is? Should I submit the patch for review anyway? Is there any other work that needs to be done first that I may contribute to? Thanks in advance for Twisted and for your feedback, -- exvito ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] Windows spawnProcess - Child process inherits files and sockets (POSIX does not)
On Thu, Mar 31, 2016 at 10:27 PM, Glyph wrote: > Before submitting a patch for review, I'm looking for preliminary > feedback, assuming you agree that the Windows vs POSIX semantics should be > the same (if not, why?). > > > After much thought: Yes. They should be the same. The reason they're not > is largely ignorance of the relevant APIs and abstractions on Windows, not > any desire that they differ. The one place they have to differ a little > bit is handle inheritance: we need to figure out some way to express the > 'childFDs' mapping in terms of both file descriptors and handles. > Agreed. My code does not go into 'childFDs' mapping territory, though. All it does is prevent all file- and socket-handles from being inherited by the child process -- other than the pipes used for STDIO communication. My patch calls a few Windows APIs via ctypes, however, as far as I can > tell, Twisted on Windows requires pywin32 and, recently, there has been > some discussion around dropping that dependency and moving towards > something based on cffi. > > > ctypes is dangerous and error-prone. If you screw up the bit-width of a > type, or the type in a header changes on some future version, ctypes gives > you no way of finding out until some poor user's process segfaults, and > usually not at the relevant call site. So we'd prefer not to maintain more > ctypes-using code. > > The APIs in pywin32 very closely mirror the underlying Windows API, so for > addressing this issue, please just go ahead and use pywin32 APIs; porting > them to a new API along with everything else should be relatively > straightforward. > > If we do move forward with that change, we will probably use > https://pypi.python.org/pypi/pywincffi and not move anything within > Twisted. > Agreed with your ctypes comment -- I've been hit by such faults which "magically" went away using cffi when coding against Windows TAPI. pywin32, unfortunatelly, does not include two Windows APIs (out of four) my code requires -- I just grepped the source for latest release I could find on SourceForge, 220. For completeness, the missing APIs are NtQuerySystemInformation [1] and NtQueryObject [2]. The others are GetHandleInformation [3] and SetHandleInformation [4]. What would you say the way forward is? Should I submit the patch for review > anyway? Is there any other work that needs to be done first that I may > contribute to? > > > Yes, just go ahead and write the patch. > Given that pywin32 does not provide two of the required APIs, maybe this issue is somewhat blocked. Adding to that is the fact that one particular API call in my code -- NtQuerySystemInformation [1] -- is being used with what seems to be an undocumented option -- SystemHandleInformation (enum = 16) -- and returning, again, an apparently undocumented data structure -- SYSTEM_HANDLE_INFORMATION. I downloaded and installed the available SDKs and WDKs (driver dev kits) from Microsoft and could not find any reference to those particular options or data structures. My code was created after much investigation on how to obtain the list of open handles for the current process. The gist of it is: - Call NtQuerySystemInformation with the SystemInformationClass arg set to SystemHandleInformation. - This returns all (!!!) of the handles in the system (no need for special privileges). - Filter those out by the current process PID and type, such that only files and sockets are left. - Use the GetHandleInformation to get the inheritance flag and clear it with SetHandleInformation if needed. It is mostly based on SysInternals information at http://forum.sysinternals.com/howto-enumerate-handles_topic18892.html. There are many other references across the web to those undocumented options and data structures. But none of those come from formal Microsoft documents, that I could find. An alternative approach, which I also tried, for completeness sake, was to try and Get/SetHandleInformation on all possible handles -- this is completely unfeasible given that handles are at least 32 bit numbers. After all of this -- including some frustration, I confess -- I decided to go ahead and create a cffi ABI-mode variation of my original patch, anyway: it passes the same tests and, much like the ctypes approach, works nicely on my environment: Win 7 32, Win 2008R2 64, Win XP 32 (!!!), Python 2.7.11 32, Twisted 16.1, cffi 1.5.2. Just for kicks I compared the performance of the ctypes vs cffi implementation: - The ctypes code runs in 0.014s - 0.016s. - The cffi code runs in 0.03s - 0.04s. This makes sense given that the code is mostly calling out to DLLs and, AFAICT, cffi does the nice extra work of validating/converting types back and forth. Wrapping up: I'm really not sure how to more forward with this: not only does pywin32 not provide the needed APIs, but also one of those APIs -- documented -- is being used in an undocumented fashion. Even though I'd love to submit a patch, I don't think we're at that p
Re: [Twisted-Python] Windows spawnProcess - Child process inherits files and sockets (POSIX does not)
On Thu, Apr 7, 2016 at 4:57 AM, Oliver Palmer wrote: > Owner of pywincffi here, I'd certainly welcome a PR or two for pywincffi > with the necessary functions/constants/etc so you don't have to manage that > code and I'd be happy to help write it too. I think the consensus is > Twisted is going to eventually replace calls into pywin32 with calls into > pywincffi rather than implement all of that code inside of Twisted itself. > I've already started doing this in a couple of places, > twisted.python.lockfile being the one I'm actively working on because it's > simpler to start with. But the code inside of dumbwin32proc.py and > _win32handleinherit.py are both high on my list to convert too so it > probably makes sense that we work on this together if you're open to it. Certainly. Thanks for reaching out. > Looking at your code, some of it could be put into pywincffi already. It > would need more tests and some additional code so the API calls are closer > to what's already in the project (type checking, default arguments, > documentation, etc) but overall it seems like you've already done the major > work of understanding how it all fits together. The other advantages of > putting this code into pywincffi is testing and releases are easier because > the project is using AppVeyor to test all PRs and build the wheel files for > most major Python versions including both 32 and 64 bit variants. From > Twisted's perspective, it's just a dependency on another library. Agreed. I'll be happy to follow your guidance in that. >> Just for kicks I compared the performance of the ctypes vs cffi >> implementation: ... > > Have you tried a comparison between out-of-line modules and those using > dlopen? I imagine they'd end up being pretty similar in the end performance > wise but I am a little curious. In pywincffi I started out using dlopen but > moved away from it because I needed to write some extra code which couldn't > be included from a header. The other advantage I saw is that you don't have > to rely on the DLL being present and/or Windows being able to locate it so > you can include code which might only be available if you have some extra > library installed. I did not and I confess that I haven't fully grasped (yet) the different cffi "approaches", if I may call them so. >> It is mostly based on SysInternals information at >> http://forum.sysinternals.com/howto-enumerate-handles_topic18892.html. There >> are many other references across the web to those undocumented options and >> data structures. But none of those come from formal Microsoft documents, >> that I could find. > > Some of the people on that site either have contacts within Microsoft or > have worked for Microsoft at one point so I usually trust what's there if > it's the only source. The other place I often look is the ReactOS project > where they've managed to reverse engineer quite a bit of the Windows kernel > which can either hint at the info you need or validate what you already > know. AFAICT, SysInternals was bought by / integrated with Microsoft: tteir tools are now available under Microsoft domains (example: Process Explorer at https://technet.microsoft.com/en-us/sysinternals/processexplorer.aspx). >> Wrapping up: I'm really not sure how to more forward with this: not only >> does pywin32 not provide the needed APIs, but also one of those APIs -- >> documented -- is being used in an undocumented fashion. > > IMHO (again, with some bias), I think implementing the calls you need in > pywincffi is the first step. If the calls are undocumented it would also be > a good place to do the necessary research, testing and development I think > in isolation from Twisted itself so it's clear we're going in the right > direction. Once that's done a patch set for Twisted, which calls into > pywincffi, can be opened and tested across the supported platforms. This > makes the patch set smaller but also makes it easier to understand what if > anything the new code breaks. Sounds like a sane and safe approach. > Regardless, even if you don't want to go the route of putting this into > pywincffi thanks for working on this because it helps in some of the work > I'm doing too. Thank you to, again. I will issue PRs against pywincffi such that the APIs are available. They most probably won't be up to pywincffi's standards/requirements but I expect we can cooperate under that context and go from there. -- exvito ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] Windows spawnProcess - Child process inherits files and sockets (POSIX does not)
> Thank you to, again. I will issue PRs against pywincffi such that the > APIs are available. They most probably won't be up to pywincffi's > standards/requirements but I expect we can cooperate under that > context and go from there. For completeness, if anyone wants to follow-along or contribute, first PR is https://github.com/opalmer/pywincffi/pull/66. Work will continue there. Once it becomes "Twisted ready" we'll bring the discussion back here. Thanks to all. -- exvito ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
[Twisted-Python] ITransport.write after IProtocol.connectionLost -- no failure/exception?
Dear all, The subject says most of it. While diagnosing a behavior on a somewhat large codebase, I found out something somewhat surprising -- see subject -- which is replicated with the minimal code example below. It is a LineReceiver client that: 1. Connects to 127.0.0.1:1. 2. Sends a line of text. 3. Disconnects one second later. 4. Tries to send another line of text, one second after disconnection. I expected step 4 to fail somehow, given that Twisted promptly detected that the connection closed and IProtocol.connectionLost was called, as documented. Nevertheless, it does not fail. So the questions are: a) Why does it not fail? I'm sure there is a good reason for that, as most things do in Twisted. b) Does a protocol implementation, like the one in the example, really need to track connectionMade / connectionLost calls before going for self.transport.write? c) Can a protocol implementation, instead, depend on self.transport.connected for which I found no documentation? (I probably missed it, where is it?) Thanks in advance for enlightening me. :-) Regards, -- exvito # --[ cut here ] from twisted.internet import protocol, reactor, defer from twisted.protocols import basic class ClientAPI(basic.LineReceiver): def _log(self, msg): print('[connected=%r] %s' % (self.transport.connected, msg)) def _send(self, line): self.sendLine(line) self._log('sent: %r' % (line,)) def connectionMade(self): self._log('connection made') self._send('first line') reactor.callLater(1, lambda: self.transport.loseConnection()) def connectionLost(self, reason): self._log('connection lost') reactor.callLater(1, self.postConnectionLostCall) def postConnectionLostCall(self): self._send('post conn lost call - SHOULD FAIL') class ClientFactory(protocol.ClientFactory): protocol = ClientAPI def clientConnectionFailed(self, connector, reason): print 'connection failed -', reason reactor.stop() if __name__ == '__main__': factory = ClientFactory() reactor.connectTCP('127.0.0.1', 1, factory) reactor.run() ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] ITransport.write after IProtocol.connectionLost -- no failure/exception?
On Fri, Dec 16, 2016 at 8:27 PM, Glyph Lefkowitz wrote: > >> On Dec 16, 2016, at 5:22 AM, Cory Benfield wrote: >> >>> On 15 Dec 2016, at 12:07, exvito here wrote: >>> >>> Dear all, >>> >>> The subject says most of it. While diagnosing a behavior on a somewhat >>> large codebase, I found out something somewhat surprising -- see >>> subject -- which is replicated with the minimal code example below. >>> >>> It is a LineReceiver client that: >>> 1. Connects to 127.0.0.1:1. >>> 2. Sends a line of text. >>> 3. Disconnects one second later. >>> 4. Tries to send another line of text, one second after disconnection. >>> >>> I expected step 4 to fail somehow, given that Twisted promptly >>> detected that the connection closed and IProtocol.connectionLost was >>> called, as documented. Nevertheless, it does not fail. >> >> I can’t speak for the design of Twisted, but this is definitely an intended >> implementation behaviour: because twisted’s write method is non-blocking, it >> is generally nicer to avoid write exploding in your face when disconnected. > > I can! The main design feature of this approach is that if you have a loop > like this: > > for x in messages: > self.transport.write(self.encode(x)) > > you should not have to write it to be: > > for x in messages: > if self._flagISetInConnectionLost: > break > self.transport.write(self.encode(x)) > > just to avoid seeing tracebacks in your logs. > > If you care about the disconnection event, implement connectionLost to do the > thing that needs to be done (in this case, stop your LoopingCall). That's > what that callback is there for! Thanks for your input Cory and Glyph. I do agree that a well written protocol should not self.transport.write after connectionLost -- it does not make any kind of sense to do that. It's just that the one I was debugging was doing it in its app-level keep alive messages triggered by IReactorTime.callLater which wasn't properly cancelled on connectionLost. This, in turn, lead to unpexpected app-level keep alive timeouts after disconnection which, while acceptable (depending on app/protocol design), were having a negative impact on the overall connection/protocol teardown and cleanup. Having said this, the fact that self.transport.write does not complain after connectionLost just made my analysis and debugging more difficult (yes, I was aware that it is non-blocking -- thanks Cory -- and that when talking to the network the only confirmation of delivery must be obtained from a received message stating that fact). All in all, I was just looking for confirmation from a conceptual / design standpoint. That was my purpose and it is now clear. Thanks. Going a little bit further, in this context, I wonder if my understanding of the Protocol / Transport objects lifecycle is 100% clear. I think there is a one to one relationship in the sense that, simply put, on an incoming connection the underlying machinery instantiates a Protocol via the Factory's buildProtocol method, instantiates a Transport object and then calls Protocol.makeConnection to "attach" the transport to the protocol; when the connection is closed, the Transport calls connectionLost on the Protocol and, at some point, those two objects are no longer "usable" -- they've done their job and will be destroyed if no app level references are kept to them. If this is correct, in particular the fact that a given Transport instance is not reusable after disconnection (if it is, could you please point out such a case?), wouldn't it be valuable to log a warning when write after disconnect is called? (agreed, raising an Exception would be too much and break compatiblity). I find two reasons for that: on one hand, given that it would have made my debugging easier, it may help other developers pin point such cases; on the other, the fact that Twisted itself logs messages on several occasions, such as, for example, the default noisy Factory on doStart / doStop. Adding to that, the fact that, if I'm reading the code correctly, ITransport.write just throws away the data for TCP transports since it falls back to FileDescriptor.write that simply returns if "not self.connected or self._writeDisconnected" in src/twisted/internet/abstract.py. Thoughts? (or corrections, of course?) Again, thanks for your time and input. Regards, PS: I don't mean to start a huge discussion on the topic or question the current design. It is very nice as it is, thank you! I'm just looking for improving my understanding and, humbly, contributing back. -- exvito ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] "disconnecting properly" in tests still hangs on macOS
> On 14 Nov 2018, at 08:50, Chris Withers wrote: > > Right, so, I've been trying to get the technique in > https://jml.io/pages/how-to-disconnect-in-twisted-really.html to work for me. > > No hating please, most of my testing in the past has involved hitting a > relational database, so there's already a TCP connection flying around, one > more won't make any difference. > > jml's example, exactly as-is on that page, hangs around 30-40% of the time > when running on my macOS laptop. From changing the teardown to look like this: > > def tearDown(self): > ds = defer.maybeDeferred(self.serverPort.stopListening) > dc = defer.maybeDeferred(self.clientConnection.disconnect) > print() > > ds.addCallback(lambda _: print('serverPort.stopListening')) > dc.addCallback(lambda _: print('self.clientConnection.disconnect')) > self.clientDisconnected.addCallback(lambda _: > print('self.clientDisconnected')) > self.serverDisconnected.addCallback(lambda _: > print('self.serverDisconnected')) > self.serverDisconnected.addErrback(lambda _: > print('self.serverDisconnected:', _)) > return defer.gatherResults([ds, dc, self.clientDisconnected, > self.serverDisconnected]) > > ...it appears that it's the serverDisconnected deferred that's failing to > fire. I can't reproduce this on Linux as of yet, so I'm guessing this is a > difference between the SelectReactor used on macOS and the EPollReactor used > on Linux. > > What's the best way to go about debugging a non-firing deferred like this? > > Anyone know what this might be? Chris, I played with this for a bit and quickly reproduced the "server side disconnect never seems to happen" behaviour you described, on my system running macOS 10.12.6 and Twisted 18.9.0 using the SelectReactor. Suspecting of an eventual race condition between "server stop listen" and "server disconnect", I tried this variation of tearDown which seems to work reliably: @defer.inlineCallbacks def tearDown(self): self.clientConnection.disconnect() yield defer.gatherResults([self.clientDisconnected, self.serverDisconnected]) yield defer.maybeDeferred(self.serverPort.stopListening) Do I have any motive to suspect such race condition? No, but after ensuring "disconnect first, stop listening later", things work much better here. Also tested the CFReactor and KQueueReactor: CFReactor seems to exhibit a similar behaviour (original code hangs every now and then on cleanup, my code works); KQueueReactor always hangs on cleanup with the original code, and works reliably with my code. My 2c., -- exvito ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com https://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python