[Twisted-Python] Twisted windows hackers - help the tests to pass!
Twisted fails scads of tests on Windows. You can see that here: http://twistedmatrix.com/buildbot/ I know that there are at least a few people on this list that use Twisted on Windows. Doesn't it bother you that it's broken? If you're a windows developer, help out and fix some tests! A few Twisted hackers, Justin J. in particular, are hard at work to make it scale and perform well on Windows, but in addition to the rocket-science of IOCP there is quite a bit of grunt work to do, especially in the area of understanding why certain tests fail when they pass on other (more UNIX-y) platforms. So please, try debugging some Twisted unit tests and either submitting bug reports explaining what's going on so an existing Twisted dev can fix them, or submitting patches that we can apply. The real bonus for you if you're using Twisted on Windows in some commercial capacity is that once the buildbot turns green, test fascists like JP (or, on off days, myself) can revert changes which break it, and enforce a higher standard of quality for portability of Twisted code. Right now it's hard to convince developers to not make it fail _more_ tests, since hey, it's broken already, right?
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Well, let's start by looking at the buildslave environment itself. http://tm.com/bb shows 21 errors. Most of them appear to be identical permission denied errors, which suggests a configuration problem particular to that buildbot installation. When I run the tests, I get: =============================================================================== [ERROR]: twisted.test.test_paths.FilePathTestCase.testOpen File "C:\Documents and Settings\cory\svn\Twisted\twisted\test\test_paths.py", line 249, in testOpen f.write('ghi') exceptions.IOError: (0, 'Error') =============================================================================== [ERROR]: twisted.test.test_ssl.ConnectionLostTestCase.testFailedVerify Failure: twisted.internet.error.ConnectionLost: Connection to the other side was lost in a non-clean fashion. - ------------------------------------------------------------------------------- Ran 891 tests in 68.844s FAILED (skips=77, expectedFailures=9, errors=2, successes=802) - - C glyph@divmod.com wrote:
Twisted fails scads of tests on Windows. You can see that here: http://twistedmatrix.com/buildbot/
I know that there are at least a few people on this list that use Twisted on Windows. Doesn't it bother you that it's broken? If you're a windows developer, help out and fix some tests! A few Twisted hackers, Justin J. in particular, are hard at work to make it scale and perform well on Windows, but in addition to the rocket-science of IOCP there is quite a bit of grunt work to do, especially in the area of understanding why certain tests fail when they pass on other (more UNIX-y) platforms.
So please, try debugging some Twisted unit tests and either submitting bug reports explaining what's going on so an existing Twisted dev can fix them, or submitting patches that we can apply.
The real bonus for you if you're using Twisted on Windows in some commercial capacity is that once the buildbot turns green, test fascists like JP (or, on off days, myself) can revert changes which break it, and enforce a higher standard of quality for portability of Twisted code. Right now it's hard to convince developers to not make it fail _more_ tests, since hey, it's broken already, right?
_______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (MingW32) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org
iD8DBQFDtguS3A5SrXAiHQcRAmDmAKClsweuv3R8x53G1TJHiUoEbiWsPwCgj2gJ CTtw71uOF3472VLA/QW4Hzk= =1tb2 -----END PGP SIGNATURE-----
On Fri, 30 Dec 2005 20:39:46 -0800, Cory Dodt <corydodt@twistedmatrix.com> wrote:
Well, let's start by looking at the buildslave environment itself. http://tm.com/bb shows 21 errors. Most of them appear to be identical permission denied errors, which suggests a configuration problem particular to that buildbot installation.
Well, that's good to know! Thanks for clearing that up. Any clue what the particular misconfiguration might be? I have never even seen that particular buildbot machine. (I can volunteer another one through VNC for the buildbot maintainer to test with if that would help.) I think there is still a generally good reason to be concerned though - quite a few tests are still set to skip on Windows that aren't on UNIX. (and the aforementioned "it's broken anyway" attitude is especially bad if, in fact, it is *that* close to working)
On 12/31/05, glyph@divmod.com <glyph@divmod.com> wrote:
On Fri, 30 Dec 2005 20:39:46 -0800, Cory Dodt <corydodt@twistedmatrix.com> wrote:
Well, let's start by looking at the buildslave environment itself. http://tm.com/bb shows 21 errors. Most of them appear to be identical permission denied errors, which suggests a configuration problem particular to that buildbot installation.
Well, that's good to know! Thanks for clearing that up. Any clue what the particular misconfiguration might be? I have never even seen that particular buildbot machine. (I can volunteer another one through VNC for the buildbot maintainer to test with if that would help.)
It's not so much a misconfiguration, as an issue with the fact that trial on Window s is failing to rename its _trial_temp folder, because there are files inside it that are open. I've opened a bug on it over on < http://twistedmatrix.com/bugs/issue1387>. It's going to need a rethink on how _trial_temp works, because my instant thought on how to solve was "symlinks" and python doesn't support them on windows, mostly because windows' own support of them is a tad on the "dont' ask, don't tell" side of things, and NTFS-only anyway. MFen tracked down an error involving "r+b" mode. Seems windows handling of it is insane. See <http://twistedmatrix.com/bugs/issue1386>. This could well be a python bug, or a feature of windows. Moof
ok, i can actually chime in here because i've done filesystems work on windows (don't ask ;). now, it's been a while, but i should remember things reasonably accurately (i hope). see below for comments: ----- Original Message ----- From: Moof To: twisted-python@twistedmatrix.com Sent: Saturday, December 31, 2005 2:15 AM Subject: Re: [Twisted-Python] Twisted windows hackers - help the tests to pass!
It's not so much a misconfiguration, as an issue with the fact that trial on Window s is failing to rename its _trial_temp folder, because there are files inside it that are open. I've opened a bug on it over on < http://twistedmatrix.com/bugs/issue1387>.
it is incorrect to state that file deletion will always fail when the file is open. let me explain how nt/ntfs/win32 does this, because it's highly 'strange' to folks from a posix background. this may be way more info than you were looking for, but i'm going to type it out anyway so that someone can then make the call on how to deal with the issue. for functionality implemented in the kernel, such as ntfs, windows has 2 api layers: win32 (you see CreateFile() here) and the native nt api. win32 is implemented in usermode, while the native api is implemented as a kernel service, which is exposed to userspace code with the Zw prefix (ie ZwCreateFile()). sometimes the win32 api calls map directly, sometimes they are multiplexed (ie one win32 api call may use multiple native api calls to do the work). what happens here is undocumented (at least officially and without an nda). what is important is that some calls allow you to do things which win32 does not expose; most popular ones deal with io completion (this is how you cancel async i/o on windows nt). now, on to the meat: the win32 CreateFile() is actually a jack-of-all-trades call (horrible design), used for file creation, opening *and authorization*. we're not interested in ACLs in this case, but we are interested in locks - locks in nt are compulsory (as opposed to purely advisory in posix) and violating one will result in an auth failure. now, disregarding the ability to take out range locks (locks on a byterange within a file), CreateFile() takes out certain locks based on the sharemode (iirc, not sure) parameter. the rules here are funky, iirc, but the one we care about is FILE_SHARE_DELETE. *unless* this flag is set in sharemode passed to CreateFile(), all attempts to open this file for deletion (see, authorization) going forward will fail. it will fail when the file is opened exclusively. CreateFile takes a parameter called sharemode, iirc, which can be a combination of FILE_SHARE_READ, FILE_SHARE_WRITE, and FILE_SHARE_DELETE. *unless* FILE_SHARE_DELETE is set in the flag parameter, a lock preventing deletion gets taken out and you get to enjoy all the wonders of compulsory locking you are hitting. here's why i think this happens: win32 DeleteFile() does *not* actually use ZwDeleteFile() native call to delete a file. instead, it does a CreateFile() (or ZwCreateFile(), could be either) to open the file (and get a handle to it), telling it the desired access is DELETE. then it does a ZwSetFileInformation(), which it tells it wants to set 'disposition' and passes in the appropriate disposition info struct with the DELETE disposition set. you can't cheat this, because ZwSetInformation() will fail if the handle doesn't have DELETE rights, and you won't get them if the file isn't opened with FILE_SHARE_DELETE. bummer. the reason this is all so roundabout is that the file whose disposition is set to DELETE doesn't actually get deleted until the last handle is ZwClose()'d - this is where the deletion takes place. now, this is from memory, so don't hold me to it exactly, but the jist of it should be correct. the interesting, albeit non-obvious, question is: what does ZwDeleteFile() do? it takes either a handle *or a path* and deletes it *right away*, without waiting for handles to be closed. now, i don't know whether it bypasses the ZwCreateFile() and hence the DELETE check, but there's a chance that it is the call ZwClose() makes internally when it does a delete based on the disposition (this guess would be supported by the fact that ZwCreateFile() and a few other fs calls are documented in msdn/ddk, but ZwDeleteFile() is not) and hence doesn't hit this check. what is even more interesting is that, in win32, the only way to remove a directory is using RemoveDirectory(), which requires the directory to be empty. you have to recursively delete all contents, either by hand or using SHFileOperation, iirc. this is the op that hits your open files problem. i remember seeing code (this part i didn't work on, but did read brielfy) which deleted directories with ZwDeleteFile() and there was no resurive content deletion code, so i suspect that you could delete a non-empty directory this way. neither of this is valid ntfs usage, so doing it may not be kosher. whatever the case may be, those two options are available if you can stomach them.
It's going to need a rethink on how _trial_temp works, because my instant thought on how to solve was "symlinks" and python doesn't support them on windows, mostly because windows' own support of them is a tad on the "dont' ask, don't tell" side of things, and NTFS-only anyway.
NTFS and DFS, but yeah, no FAT (if anyone cares). symlinks in ntfs can be implemented using what's called 'reparse points'. these are actually quite powerful - you can attach either a static transformation or code (you need a driver for this, iirc) to a certain dentry these symlinks are called 'junctions', but they work only for directories. currently, all of this is extremely hairy to use. there are also hardlinks, which you can actually create with the win32 api, but they are only for files moreover, none of this behaves like symlinks and hardlinks in terms of finer semantics (ie unlinking). junctions are most closely related to mount --bind, rather than symlinks, for example. with all of the above said, you've basically got these choices (in no particular order) to deal with the issue at hand: 1. change the offending code not to do this rename 2. instead of a rename, create a new directory, do a recursive copy into it from the original and retry removing the original directory asynchronously until it succeeds. obviously, the file handles which are open at that point will be referencing a different copy of the files from the ones which will be opened subsequently. 3. test whether the zwDeleteFile() behaves in the way i conjrectured it to (wrt files or directories). if so, cause it to be implemented in the pywin32 extension and use it to perform the delete. 4. test whether you can either use ZwSetFileInformation() to rename directories by changing the FILE_NAME attr in the appropriate info structure or use it to move by renaming files which are open, again using the appropriate (but different) structure. if so, implement this or cause this to be implemented in pywin32. it is unclear (to me) whether this would result in the pre-move file handles being dead, stale or correct. 5. instead of opening the files as normal, open them with pywin32's implementation of CreateFile(), specifying the appropriate sharemode. this will allow the rename (move really) to go through, but it is unclear what happens to the preexisting filehandles. 6. implement, or cause to be implemented, CreateHardlink() in pywin32. create a new directory and recursively hardlink contents of the original into the new directory. asynchronously retry recursive deletion of the original one. that's all i can think of anyway.
MFen tracked down an error involving "r+b" mode. Seems windows handling of it is insane. See ><http://twistedmatrix.com/bugs/issue1386>. This could well be a python bug, or a feature of windows.
i'm not sure how python does file opens and i/o on windows. with that said, assuming that it uses fopen() from the visual studio c runtime library, there is a quirk in the implementation that might be causing this. if you use any of the + modes, ie a+, r+ or w+. when you switch between reading and writing you need to do an fflush() or fsetpos() (possibly some others like fseek() could work too, don't remember). try doing a file.flush() on your file object somewhere in there and see if that fixes things for you. if it does, this should probably be reported as a python stdlib bug. it's really late, so pardon the wordiness and possible inaccuracies due to memory lapses. hth, -p
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Paul, flush fixes one kind of insanity and seek fixes another: One =========== $ echo abcdef > foo $ python
f = file('foo', 'r+b') f.write('ghi') f.flush() f.read() 'def\n' # hooray!
Two =========== $ echo abcdef > foo $ python
f = file('foo','r+b') f.read() 'abcdef\n' f.seek(7, 0) f.write('xyz') ^D $ cat foo abcdef xyz $ # hooray
Paul - Do you still think this is a Python bug? Does anyone know if this has already been discussed on pyml? C Paul G wrote:
ok, i can actually chime in here because i've done filesystems work on windows (don't ask ;). now, it's been a while, but i should remember things reasonably accurately (i hope). see below for comments:
----- Original Message ----- From: Moof To: twisted-python@twistedmatrix.com Sent: Saturday, December 31, 2005 2:15 AM Subject: Re: [Twisted-Python] Twisted windows hackers - help the tests to pass!
It's not so much a misconfiguration, as an issue with the fact that trial on Window s is failing to rename its _trial_temp folder, because there are files inside it that are open. I've opened a bug on it over on < http://twistedmatrix.com/bugs/issue1387>.
it is incorrect to state that file deletion will always fail when the file is open. let me explain how nt/ntfs/win32 does this, because it's highly 'strange' to folks from a posix background. this may be way more info than you were looking for, but i'm going to type it out anyway so that someone can then make the call on how to deal with the issue.
for functionality implemented in the kernel, such as ntfs, windows has 2 api layers: win32 (you see CreateFile() here) and the native nt api. win32 is implemented in usermode, while the native api is implemented as a kernel service, which is exposed to userspace code with the Zw prefix (ie ZwCreateFile()). sometimes the win32 api calls map directly, sometimes they are multiplexed (ie one win32 api call may use multiple native api calls to do the work). what happens here is undocumented (at least officially and without an nda). what is important is that some calls allow you to do things which win32 does not expose; most popular ones deal with io completion (this is how you cancel async i/o on windows nt).
now, on to the meat: the win32 CreateFile() is actually a jack-of-all-trades call (horrible design), used for file creation, opening *and authorization*. we're not interested in ACLs in this case, but we are interested in locks - locks in nt are compulsory (as opposed to purely advisory in posix) and violating one will result in an auth failure. now, disregarding the ability to take out range locks (locks on a byterange within a file), CreateFile() takes out certain locks based on the sharemode (iirc, not sure) parameter. the rules here are funky, iirc, but the one we care about is FILE_SHARE_DELETE. *unless* this flag is set in sharemode passed to CreateFile(), all attempts to open this file for deletion (see, authorization) going forward will fail.
it will fail when the file is opened exclusively. CreateFile takes a parameter called sharemode, iirc, which can be a combination of FILE_SHARE_READ, FILE_SHARE_WRITE, and FILE_SHARE_DELETE. *unless* FILE_SHARE_DELETE is set in the flag parameter, a lock preventing deletion gets taken out and you get to enjoy all the wonders of compulsory locking you are hitting.
here's why i think this happens: win32 DeleteFile() does *not* actually use ZwDeleteFile() native call to delete a file. instead, it does a CreateFile() (or ZwCreateFile(), could be either) to open the file (and get a handle to it), telling it the desired access is DELETE. then it does a ZwSetFileInformation(), which it tells it wants to set 'disposition' and passes in the appropriate disposition info struct with the DELETE disposition set. you can't cheat this, because ZwSetInformation() will fail if the handle doesn't have DELETE rights, and you won't get them if the file isn't opened with FILE_SHARE_DELETE. bummer. the reason this is all so roundabout is that the file whose disposition is set to DELETE doesn't actually get deleted until the last handle is ZwClose()'d - this is where the deletion takes place. now, this is from memory, so don't hold me to it exactly, but the jist of it should be correct.
the interesting, albeit non-obvious, question is: what does ZwDeleteFile() do? it takes either a handle *or a path* and deletes it *right away*, without waiting for handles to be closed. now, i don't know whether it bypasses the ZwCreateFile() and hence the DELETE check, but there's a chance that it is the call ZwClose() makes internally when it does a delete based on the disposition (this guess would be supported by the fact that ZwCreateFile() and a few other fs calls are documented in msdn/ddk, but ZwDeleteFile() is not) and hence doesn't hit this check. what is even more interesting is that, in win32, the only way to remove a directory is using RemoveDirectory(), which requires the directory to be empty. you have to recursively delete all contents, either by hand or using SHFileOperation, iirc. this is the op that hits your open files problem. i remember seeing code (this part i didn't work on, but did read brielfy) which deleted directories with ZwDeleteFile() and there was no resurive content deletion code, so i suspect that you could delete a non-empty directory this way. neither of this is valid ntfs usage, so doing it may not be kosher.
whatever the case may be, those two options are available if you can stomach them.
It's going to need a rethink on how _trial_temp works, because my instant thought on how to solve was "symlinks" and python doesn't support them on windows, mostly because windows' own support of them is a tad on the "dont' ask, don't tell" side of things, and NTFS-only anyway.
NTFS and DFS, but yeah, no FAT (if anyone cares). symlinks in ntfs can be implemented using what's called 'reparse points'. these are actually quite powerful - you can attach either a static transformation or code (you need a driver for this, iirc) to a certain dentry these symlinks are called 'junctions', but they work only for directories. currently, all of this is extremely hairy to use. there are also hardlinks, which you can actually create with the win32 api, but they are only for files moreover, none of this behaves like symlinks and hardlinks in terms of finer semantics (ie unlinking). junctions are most closely related to mount --bind, rather than symlinks, for example.
with all of the above said, you've basically got these choices (in no particular order) to deal with the issue at hand:
1. change the offending code not to do this rename 2. instead of a rename, create a new directory, do a recursive copy into it from the original and retry removing the original directory asynchronously until it succeeds. obviously, the file handles which are open at that point will be referencing a different copy of the files from the ones which will be opened subsequently. 3. test whether the zwDeleteFile() behaves in the way i conjrectured it to (wrt files or directories). if so, cause it to be implemented in the pywin32 extension and use it to perform the delete. 4. test whether you can either use ZwSetFileInformation() to rename directories by changing the FILE_NAME attr in the appropriate info structure or use it to move by renaming files which are open, again using the appropriate (but different) structure. if so, implement this or cause this to be implemented in pywin32. it is unclear (to me) whether this would result in the pre-move file handles being dead, stale or correct. 5. instead of opening the files as normal, open them with pywin32's implementation of CreateFile(), specifying the appropriate sharemode. this will allow the rename (move really) to go through, but it is unclear what happens to the preexisting filehandles. 6. implement, or cause to be implemented, CreateHardlink() in pywin32. create a new directory and recursively hardlink contents of the original into the new directory. asynchronously retry recursive deletion of the original one.
that's all i can think of anyway.
<http://twistedmatrix.com/bugs/issue1386>. This could well be a
MFen tracked down an error involving "r+b" mode. Seems windows handling of it is insane. See python bug, or a feature of windows.
i'm not sure how python does file opens and i/o on windows. with that said, assuming that it uses fopen() from the visual studio c runtime library, there is a quirk in the implementation that might be causing this. if you use any of the + modes, ie a+, r+ or w+. when you switch between reading and writing you need to do an fflush() or fsetpos() (possibly some others like fseek() could work too, don't remember). try doing a file.flush() on your file object somewhere in there and see if that fixes things for you. if it does, this should probably be reported as a python stdlib bug.
it's really late, so pardon the wordiness and possible inaccuracies due to memory lapses.
hth, -p
_______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
-----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (MingW32) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFDttEp3A5SrXAiHQcRAq4gAJ4pZQS+vFSlKqS8RvJe8ialmzANEgCfe4BL G4OrBEocHxEcB3LD3tpr1OY= =XA5K -----END PGP SIGNATURE-----
----- Original Message ----- From: "Cory Dodt" <corydodt@twistedmatrix.com> To: "Twisted general discussion" <twisted-python@twistedmatrix.com> Sent: Saturday, December 31, 2005 1:46 PM Subject: [Twisted-Python] Re: plus mode was Re: how winnt fileops work and what to do about it
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Paul, flush fixes one kind of insanity and seek fixes another:
One =========== $ echo abcdef > foo $ python
f = file('foo', 'r+b') f.write('ghi') f.flush() f.read() 'def\n' # hooray!
Two =========== $ echo abcdef > foo $ python
f = file('foo','r+b') f.read() 'abcdef\n' f.seek(7, 0) f.write('xyz') ^D $ cat foo abcdef xyz $ # hooray
Paul - Do you still think this is a Python bug?
depends on what you call a bug. this behaviour is, afaik, specific to microsoft libc's (aka c runtime) implementation of file streams. since their implementation exhibits arcanely and somewhat subtly (subtle in terms of cause, as opposed to effect - the effect is quite apparent) divergent behaviour, it definitely violates the users' expectations, so it is a bug in that sense. with that said, they never claimed to be $standard compliant and they do document this, iirc, so it is not quite a bug in that sense. in practical terms, their implementation is not likely to 'get fixed', so the failure to deal with this quirk is a python bug. there are two options for python: 1. say that python expects users to be aware of quirks on specific platforms and the responsibility of dealing with them lies with the user. in this case, this quirk needs to be mentioned in the python docs, since it is not reasonable to expect people to figure out that they need to look at msdn c runtime api docs. 2. say that across all platforms python supports given functionality on, said functionality needs to have functionally identical results given identical input. in this case, the flush+seek workaround needs to be utilized on all cpythons linked with msvscrt in a manner transparent to the user. make sense? -p
On Sat, 31 Dec 2005 15:21:45 -0500, Paul G <paul-lists@perforge.com> wrote:
2. say that across all platforms python supports given functionality on, said functionality needs to have functionally identical results given identical input. in this case, the flush+seek workaround needs to be utilized on all cpythons linked with msvscrt in a manner transparent to the user.
It seems like we can work around this more easily than that, considering that flush and seek are available from Twisted; the file object causing problems in the tests is being returned from the open() method of a FilePath object, if I understand it correctly. FilePath could include the workaround far in advance of Python deciding to.
On Sat, 31 Dec 2005 16:33:46 -0500, glyph@divmod.com wrote:
On Sat, 31 Dec 2005 15:21:45 -0500, Paul G <paul-lists@perforge.com> wrote:
2. say that across all platforms python supports given functionality on, said functionality needs to have functionally identical results given identical input. in this case, the flush+seek workaround needs to be utilized on all cpythons linked with msvscrt in a manner transparent to the user.
It seems like we can work around this more easily than that, considering that flush and seek are available from Twisted; the file object causing problems in the tests is being returned from the open() method of a FilePath object, if I understand it correctly. FilePath could include the workaround far in advance of Python deciding to.
Quite so. I definitely think this is a bug in Python though. Someone should open a ticket in the Python bug tracker. Cory? Jean-Paul
----- Original Message ----- From: "Jean-Paul Calderone" <exarkun@divmod.com> To: "Twisted general discussion" <twisted-python@twistedmatrix.com> Sent: Saturday, December 31, 2005 4:41 PM Subject: Re: [Twisted-Python] Re: plus mode was Re: how winnt fileops workand what to do about it
On Sat, 31 Dec 2005 16:33:46 -0500, glyph@divmod.com wrote:
On Sat, 31 Dec 2005 15:21:45 -0500, Paul G <paul-lists@perforge.com> wrote:
2. say that across all platforms python supports given functionality on, said functionality needs to have functionally identical results given identical input. in this case, the flush+seek workaround needs to be utilized on all cpythons linked with msvscrt in a manner transparent to the user.
It seems like we can work around this more easily than that, considering that flush and seek are available from Twisted; the file object causing problems in the tests is being returned from the open() method of a FilePath object, if I understand it correctly. FilePath could include the workaround far in advance of Python deciding to.
Quite so. I definitely think this is a bug in Python though. Someone should open a ticket in the Python bug tracker. Cory?
cory, i'll be happy to see if i can dig up an msdn entry documenting this so you can include it in the bug report. ping me if you need me to do it. -p
----- Original Message ----- From: <glyph@divmod.com> To: "Twisted general discussion" <twisted-python@twistedmatrix.com> Sent: Saturday, December 31, 2005 4:33 PM Subject: Re: [Twisted-Python] Re: plus mode was Re: how winnt fileops workand what to do about it
On Sat, 31 Dec 2005 15:21:45 -0500, Paul G <paul-lists@perforge.com> wrote:
2. say that across all platforms python supports given functionality on, said functionality needs to have functionally identical results given identical input. in this case, the flush+seek workaround needs to be utilized on all cpythons linked with msvscrt in a manner transparent to the user.
It seems like we can work around this more easily than that, considering that flush and seek are available from Twisted; the file object causing problems in the tests is being returned from the open() method of a FilePath object, if I understand it correctly. FilePath could include the workaround far in advance of Python deciding to.
oh, it makes perfect sense to have a workaround in twisted, i'm not suggesting otherwise. however, it should be just that - a temporary workaround until cpython has a fix for the issue, because (outside of the offending libc implementation), that is the right place to fix it. -p
----- Original Message ----- From: "Paul G" <paul-lists@perforge.com> To: "Twisted general discussion" <twisted-python@twistedmatrix.com> Sent: Saturday, December 31, 2005 4:57 PM Subject: Re: [Twisted-Python] Re: plus mode was Re: how winnt fileops workandwhat to do about it
----- Original Message ----- From: <glyph@divmod.com> To: "Twisted general discussion" <twisted-python@twistedmatrix.com> Sent: Saturday, December 31, 2005 4:33 PM Subject: Re: [Twisted-Python] Re: plus mode was Re: how winnt fileops workand what to do about it
On Sat, 31 Dec 2005 15:21:45 -0500, Paul G <paul-lists@perforge.com> wrote:
2. say that across all platforms python supports given functionality on, said functionality needs to have functionally identical results given identical input. in this case, the flush+seek workaround needs to be utilized on all cpythons linked with msvscrt in a manner transparent to the user.
It seems like we can work around this more easily than that, considering that flush and seek are available from Twisted; the file object causing problems in the tests is being returned from the open() method of a FilePath object, if I understand it correctly. FilePath could include the workaround far in advance of Python deciding to.
oh, it makes perfect sense to have a workaround in twisted, i'm not suggesting otherwise. however, it should be just that - a temporary workaround until cpython has a fix for the issue, because (outside of the offending libc implementation), that is the right place to fix it.
just to clarify: there was no question to the effect of 'should we fix this in twisted for now?' - i was presuming that this course of action would be obvious and, indeed, the one being taken. cory specifically asked where i thought the bug was, so i explained my take on that. more importantly, it just occurred to me that even if the next minor cpython release contains this fix, we (and all python users) would still be stuck with this issue in previous releases. twisted would have to contain the workaround until the last version of cpython without the fix became unsupported, which could be quite a while. in light of this, not only should python be fixed, but this behaviour needs to be explained in the respective python docs quite prominently as well. -p
On Dec 31, 2005, at 4:57 PM, Paul G wrote:
oh, it makes perfect sense to have a workaround in twisted, i'm not suggesting otherwise. however, it should be just that - a temporary workaround until cpython has a fix for the issue, because (outside of the offending libc implementation), that is the right place to fix it.
As I wrote in the bug report:
It's not a python bug except perhaps that python should raise an exception. Doing a read and write without a seek in between is illegal in C.
"ANSI C requires that a file positioning function intervene between output and input, unless an input operation encounters end-of-file."
I am against including any sort of workaround besides raising an exception if the situation occurs. And I don't think even that is particularly warranted. James
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Fair enough. In that case, the test itself is broken, since it doesn't do seeks or flushes. C James Y Knight wrote:
On Dec 31, 2005, at 4:57 PM, Paul G wrote:
oh, it makes perfect sense to have a workaround in twisted, i'm not suggesting otherwise. however, it should be just that - a temporary workaround until cpython has a fix for the issue, because (outside of the offending libc implementation), that is the right place to fix it.
As I wrote in the bug report:
It's not a python bug except perhaps that python should raise an exception. Doing a read and write without a seek in between is illegal in C.
"ANSI C requires that a file positioning function intervene between output and input, unless an input operation encounters end-of-file."
I am against including any sort of workaround besides raising an exception if the situation occurs. And I don't think even that is particularly warranted.
James
_______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
-----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (MingW32) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFDtxVu3A5SrXAiHQcRApqwAJ0Ywaxg9qxs2QZQxB8NIILUFnTXUgCfaNHl QVcMaluKYVx0cf5+mWHo2mU= =9NK8 -----END PGP SIGNATURE-----
On Sat, 31 Dec 2005 18:12:06 -0500, James Y Knight <foom@fuhm.net> wrote:
On Dec 31, 2005, at 4:57 PM, Paul G wrote:
oh, it makes perfect sense to have a workaround in twisted, i'm not suggesting otherwise. however, it should be just that - a temporary workaround until cpython has a fix for the issue, because (outside of the offending libc implementation), that is the right place to fix it.
As I wrote in the bug report:
It's not a python bug except perhaps that python should raise an exception. Doing a read and write without a seek in between is illegal in C.
"ANSI C requires that a file positioning function intervene between output and input, unless an input operation encounters end-of-file."
I am against including any sort of workaround besides raising an exception if the situation occurs. And I don't think even that is particularly warranted.
Thanks for pointing this out. The immediate fix is now obvious: the test needs to change. It _might_ be nice to have an assertion for this behavior, but I'm not about to add a file wrapper just for this (if all of Twisted used FilePath, instead of opening files directly, I might grant more weight to this, but since it doesn't...). Jean-Paul
----- Original Message ----- From: "Jean-Paul Calderone" <exarkun@divmod.com> To: "Twisted general discussion" <twisted-python@twistedmatrix.com> Sent: Saturday, December 31, 2005 6:46 PM Subject: Re: [Twisted-Python] Re: plus mode was Re: how winnt fileops workandwhat to do about it
On Sat, 31 Dec 2005 18:12:06 -0500, James Y Knight <foom@fuhm.net> wrote:
On Dec 31, 2005, at 4:57 PM, Paul G wrote:
oh, it makes perfect sense to have a workaround in twisted, i'm not suggesting otherwise. however, it should be just that - a temporary workaround until cpython has a fix for the issue, because (outside of the offending libc implementation), that is the right place to fix it.
As I wrote in the bug report:
It's not a python bug except perhaps that python should raise an exception. Doing a read and write without a seek in between is illegal in C.
"ANSI C requires that a file positioning function intervene between output and input, unless an input operation encounters end-of-file."
I am against including any sort of workaround besides raising an exception if the situation occurs. And I don't think even that is particularly warranted.
Thanks for pointing this out. The immediate fix is now obvious: the test needs to change. It _might_ be nice to have an assertion for this behavior, but I'm not about to add a file wrapper just for this (if all of Twisted used FilePath, instead of opening files directly, I might grant more weight to this, but since it doesn't...).
are we sure streams are only used with File objects FilePath gives us? -p
----- Original Message ----- From: "Paul G" <paul-lists@perforge.com> To: "Twisted general discussion" <twisted-python@twistedmatrix.com> Sent: Saturday, December 31, 2005 6:56 PM Subject: Re: [Twisted-Python] Re: plus mode was Re: how winnt fileopsworkandwhat to do about it
----- Original Message ----- From: "Jean-Paul Calderone" <exarkun@divmod.com> To: "Twisted general discussion" <twisted-python@twistedmatrix.com> Sent: Saturday, December 31, 2005 6:46 PM Subject: Re: [Twisted-Python] Re: plus mode was Re: how winnt fileops workandwhat to do about it
On Sat, 31 Dec 2005 18:12:06 -0500, James Y Knight <foom@fuhm.net> wrote:
On Dec 31, 2005, at 4:57 PM, Paul G wrote:
oh, it makes perfect sense to have a workaround in twisted, i'm not suggesting otherwise. however, it should be just that - a temporary workaround until cpython has a fix for the issue, because (outside of the offending libc implementation), that is the right place to fix it.
As I wrote in the bug report:
It's not a python bug except perhaps that python should raise an exception. Doing a read and write without a seek in between is illegal in C.
"ANSI C requires that a file positioning function intervene between output and input, unless an input operation encounters end-of-file."
I am against including any sort of workaround besides raising an exception if the situation occurs. And I don't think even that is particularly warranted.
Thanks for pointing this out. The immediate fix is now obvious: the test needs to change. It _might_ be nice to have an assertion for this behavior, but I'm not about to add a file wrapper just for this (if all of Twisted used FilePath, instead of opening files directly, I might grant more weight to this, but since it doesn't...).
are we sure streams are only used with File objects FilePath gives us?
the relevant snippet of an ltrace python -c "f=open('/etc/hosts/', 'r+')" --- __ctype_b_loc(0x40115740, 0x40115548, 5, 0x08112740, 0) = 0x401138e4 malloc(11) = 0x0814cd18 realloc(NULL, 16) = 0x08157910 memcpy(0x0814cd18, "/etc/hosts", 11) = 0x0814cd18 free(0x08157910) = <void> __ctype_b_loc(0x40115740, 0x40115548, 0, 0, 0) = 0x401138e4 memcpy(0x40160c74, "r+", 3) = 0x40160c74 strchr("r+", 'b') = NULL strchr("r+", 'U') = NULL __errno_location() = 0x401138e0 fopen64("/etc/hosts", "r+") = 0x08175538 --- sure enough, fopen. -p
----- Original Message ----- From: "James Y Knight" <foom@fuhm.net> To: "Twisted general discussion" <twisted-python@twistedmatrix.com> Sent: Saturday, December 31, 2005 6:12 PM Subject: Re: [Twisted-Python] Re: plus mode was Re: how winnt fileops workandwhat to do about it
On Dec 31, 2005, at 4:57 PM, Paul G wrote:
oh, it makes perfect sense to have a workaround in twisted, i'm not suggesting otherwise. however, it should be just that - a temporary workaround until cpython has a fix for the issue, because (outside of the offending libc implementation), that is the right place to fix it.
As I wrote in the bug report:
It's not a python bug except perhaps that python should raise an exception. Doing a read and write without a seek in between is illegal in C.
"ANSI C requires that a file positioning function intervene between output and input, unless an input operation encounters end-of-file."
you stop quoting right at the relevant part (ie, where the difference between eg glibc and msvcrt comes in): "(If this condition is not met, then a read is allowed to return the result of writes other than the most recent.)" does msvcrt do this? no. moreover, msvcrt needs an *fsetpos()*/fseek() and an *fflush()* (as per cory), whereas glibc will take fseek() or *fgetpos()*. this means that calls resulting in synchronization in glibc and msvcrt are different, with glibc sticking to ansi c and msvcrt deviating. there is an obvious deviation both from ansi c and from glibc usage. moreover, none of this seems to be necessary with glibc in practice (and cpython seems to not be doing it or suggesting it in the docs), since this test doesn't fail on linux. different results given the same input is a bug, imo. let us step back for a second: the problem rears its head in a file object "being returned from the open() method of a FilePath object". the user has no way to know that he should be syncing the stream, primarily because he's being given a 'File', not a 'File stream'. either these issues should be listed in the docs, or taken care of underneath, keeping a File a black box. from a design point of view, the stream is an implementation detail - exposing it is ugly. a matter of how you view it though. -p
----- Original Message ----- From: "Paul G" <paul-lists@perforge.com> To: "Twisted general discussion" <twisted-python@twistedmatrix.com> Sent: Saturday, December 31, 2005 6:50 PM Subject: Re: [Twisted-Python] Re: plus mode was Re: how winnt fileopsworkandwhat to do about it
----- Original Message ----- From: "James Y Knight" <foom@fuhm.net> To: "Twisted general discussion" <twisted-python@twistedmatrix.com> Sent: Saturday, December 31, 2005 6:12 PM Subject: Re: [Twisted-Python] Re: plus mode was Re: how winnt fileops workandwhat to do about it
On Dec 31, 2005, at 4:57 PM, Paul G wrote:
oh, it makes perfect sense to have a workaround in twisted, i'm not suggesting otherwise. however, it should be just that - a temporary workaround until cpython has a fix for the issue, because (outside of the offending libc implementation), that is the right place to fix it.
As I wrote in the bug report:
It's not a python bug except perhaps that python should raise an exception. Doing a read and write without a seek in between is illegal in C.
"ANSI C requires that a file positioning function intervene between output and input, unless an input operation encounters end-of-file."
you stop quoting right at the relevant part (ie, where the difference between eg glibc and msvcrt comes in):
"(If this condition is not met, then a read is allowed to return the result of writes other than the most recent.)"
does msvcrt do this? no. moreover, msvcrt needs an *fsetpos()*/fseek() and an *fflush()* (as per cory), whereas glibc will take fseek() or *fgetpos()*. this means that calls resulting in synchronization in glibc and msvcrt are different, with glibc sticking to ansi c and msvcrt deviating.
there is an obvious deviation both from ansi c and from glibc usage.
moreover, none of this seems to be necessary with glibc in practice (and cpython seems to not be doing it or suggesting it in the docs), since this test doesn't fail on linux. different results given the same input is a bug, imo.
let us step back for a second: the problem rears its head in a file object "being returned from the open() method of a FilePath object". the user has no way to know that he should be syncing the stream, primarily because he's being given a 'File', not a 'File stream'. either these issues should be listed in the docs, or taken care of underneath, keeping a File a black box. from a design point of view, the stream is an implementation detail - exposing it is ugly.
a matter of how you view it though.
talking to myself: no it isn't. here's the msdn article: http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vclib/html/... see the difference to the excerpt you quoted from fopen(2)? yep, no 'you dont have to sync if input op hits eof' exception. this is what's causing the breakage with msvcrt but not glibc, i bet. still think it's not a bug/is a bug in the test? i'll go sleep now, so if i've missed the boat again, i won't be here to witness the smackdown <g>. -p
On Sat, 31 Dec 2005 05:48:43 -0500, Paul G <paul-lists@perforge.com> wrote:
ok, i can actually chime in here because i've done filesystems work on windows (don't ask ;). now, it's been a while, but i should remember things reasonably accurately (i hope). see below for comments:
Thanks, Paul, for these comments. This cleared up a lot about how the filesystem works on Win32 for me.
[snip]
1. change the offending code not to do this rename 2. instead of a rename, create a new directory, do a recursive copy into it from the original and retry removing the original directory asynchronously until it succeeds. obviously, the file handles which are open at that point will be referencing a different copy of the files from the ones which will be opened subsequently. 3. test whether the zwDeleteFile() behaves in the way i conjrectured it to (wrt files or directories). if so, cause it to be implemented in the pywin32 extension and use it to perform the delete. 4. test whether you can either use ZwSetFileInformation() to rename directories by changing the FILE_NAME attr in the appropriate info structure or use it to move by renaming files which are open, again using the appropriate (but different) structure. if so, implement this or cause this to be implemented in pywin32. it is unclear (to me) whether this would result in the pre-move file handles being dead, stale or correct. 5. instead of opening the files as normal, open them with pywin32's implementation of CreateFile(), specifying the appropriate sharemode. this will allow the rename (move really) to go through, but it is unclear what happens to the preexisting filehandles. 6. implement, or cause to be implemented, CreateHardlink() in pywin32. create a new directory and recursively hardlink contents of the original into the new directory. asynchronously retry recursive deletion of the original one.
To these, I would insert a few questions to be answered first: 0. Are the test_output and test_runner tests attempting to move the top-level _trial_temp, or an identically named directory somewhere inside it? 1. Why are the tests trying to move the _trial_temp directory aside at all? 2. If there is a legitimate reason for #1, what files remain open in _trial_temp which are preventing the move from succeeding? Jean-Paul
----- Original Message ----- From: "Jean-Paul Calderone" <exarkun@divmod.com> To: "Twisted general discussion" <twisted-python@twistedmatrix.com> Sent: Saturday, December 31, 2005 4:52 PM Subject: Re: how winnt fileops work and what to do about it (was Re:[Twisted-Python] Twisted windows hackers - help the tests to pass!)
On Sat, 31 Dec 2005 05:48:43 -0500, Paul G <paul-lists@perforge.com> wrote:
ok, i can actually chime in here because i've done filesystems work on windows (don't ask ;). now, it's been a while, but i should remember things reasonably accurately (i hope). see below for comments:
Thanks, Paul, for these comments. This cleared up a lot about how the filesystem works on Win32 for me.
just a quick revision from me, for the benefit of posterity only, since investigating the feasibility of fixing the underlying cause (files being held open) seems to be the best course of action.
4. test whether you can either use ZwSetFileInformation() to rename directories by changing the FILE_NAME attr in the appropriate info structure or use it to move by renaming files which are open, again using the appropriate (but different) structure.
this will definitely not work for files based on the ddk docs i managed to dig out, and will almost certainly not work for directories (though this isn't documented either way). quite simply, expecting this to work is an expectation borne out of familiarity with things like ext2/linux-vfs, where all filesystem objects are inodes mapped into a namespace with dentries. it appears that no matter how you slice it, the underlying implementation of ntfs is drastically different, so a rename is a move and a move is a copy+delete, which brings us back to our problem. -p
participants (6)
-
Cory Dodt -
glyph＠divmod.com -
James Y Knight -
Jean-Paul Calderone -
Moof -
Paul G