<div dir="ltr">On Thu, Mar 31, 2016 at 10:27 PM, Glyph <span dir="ltr"><<a href="mailto:glyph@twistedmatrix.com" target="_blank">glyph@twistedmatrix.com</a>></span> wrote:<div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><span class=""><blockquote type="cite"><div><div dir="ltr"><div><div>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?).</div></div></div></div></blockquote><div><br></div></span><div>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.</div></div></div></blockquote><div><br></div><div>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.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><span class=""><blockquote type="cite"><div><div dir="ltr"><div><div>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.</div></div></div></div></blockquote><div><br></div></span>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.</div><div><br></div><div>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.</div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><br></div><div>If we do move forward with that change, we will probably use <a href="https://pypi.python.org/pypi/pywincffi" target="_blank">https://pypi.python.org/pypi/pywincffi</a> and not move anything within Twisted.</div></div></blockquote><div><br></div><div>Agreed with your ctypes comment -- I've been hit by such faults which "magically" went away using cffi when coding against Windows TAPI.</div><div><br></div><div>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.</div><div><br></div><div>For completeness, the missing APIs are NtQuerySystemInformation [1] and NtQueryObject [2].</div><div>The others are GetHandleInformation [3] and SetHandleInformation [4].</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><span class=""><blockquote type="cite"><div><div dir="ltr"><div><div>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?</div></div></div></div></blockquote><div><br></div></span><div>Yes, just go ahead and write the patch.</div></div></div></blockquote><div><br></div><div>Given that pywin32 does not provide two of the required APIs, maybe this issue is somewhat blocked.</div><div><br></div><div>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.</div></div><div class="gmail_extra"><br></div>My code was created after much investigation on how to obtain the list of open handles for the current process.</div><div class="gmail_extra"><div class="gmail_extra">The gist of it is:</div><div class="gmail_extra">- Call NtQuerySystemInformation with the SystemInformationClass arg set to SystemHandleInformation.</div><div class="gmail_extra">- This returns all (!!!) of the handles in the system (no need for special privileges).</div><div class="gmail_extra">- Filter those out by the current process PID and type, such that only files and sockets are left.</div><div class="gmail_extra">- Use the GetHandleInformation to get the inheritance flag and clear it with SetHandleInformation if needed.<br clear="all"><div><br></div></div></div><div class="gmail_extra">It is mostly based on SysInternals information at <a href="http://forum.sysinternals.com/howto-enumerate-handles_topic18892.html">http://forum.sysinternals.com/howto-enumerate-handles_topic18892.html</a>. 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.</div><div class="gmail_extra"><br></div><div class="gmail_extra">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.</div><div class="gmail_extra"><br></div><div class="gmail_extra"><div><div>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.</div><div><br></div><div>Just for kicks I compared the performance of the ctypes vs cffi implementation:</div><div>- The ctypes code runs in 0.014s - 0.016s.</div><div>- The cffi code runs in 0.03s - 0.04s.</div><div><br></div><div>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.</div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"></div></blockquote><div>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.</div></div><div><br></div><div>Even though I'd love to submit a patch, I don't think we're at that point yet. However, for posterity's sake and if anyone wants to take a look at the code, it is avalable at <a href="https://github.com/exvito/twisted">https://github.com/exvito/twisted</a> in branches win32-fix-handle-inherit-cffi and win32-fix-handle-inherit-ctypes. They add two tests to twisted/test/test_process.py, one line to twisted/internet/_dumbwin32proc.py and one module named twisted/internet/_win32handleinherit.py</div><div><br></div><div>I'd love to hear feedback or ideas on this.<br></div><div>Thanks again</div><div><br></div><div>[1] <a href="https://msdn.microsoft.com/en-us/library/windows/desktop/ms724509(v=vs.85).aspx">https://msdn.microsoft.com/en-us/library/windows/desktop/ms724509(v=vs.85).aspx</a></div><div>[2] <a href="https://msdn.microsoft.com/en-us/library/bb432383(v=vs.85).aspx">https://msdn.microsoft.com/en-us/library/bb432383(v=vs.85).aspx</a></div><div>[3] <a href="https://msdn.microsoft.com/en-us/library/windows/desktop/ms724329(v=vs.85).aspx">https://msdn.microsoft.com/en-us/library/windows/desktop/ms724329(v=vs.85).aspx</a></div><div>[4] <a href="https://msdn.microsoft.com/en-us/library/windows/desktop/ms724935(v=vs.85).aspx">https://msdn.microsoft.com/en-us/library/windows/desktop/ms724935(v=vs.85).aspx</a></div><div class="gmail_signature"><div dir="ltr"><div>--<br>exvito</div></div></div>
</div></div>