| msg84798 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) *  |
Date: 2009-03-31 15:24 |
| Python trunk, compiled with VS2005 SP1, release build on Windows 2000: >>> import os >>> fd = os.open("t", 0) >>> os.close(fd) Traceback (most recent call last): File "", line 1, in OSError: [Errno 9] Bad file descriptor The _PyVerify_fd() returned False for the given fd! Needless to say that there are many other similar failures. For example, subprocess does not work. Digging inside assembly code, I noticed that the __pioinfo structure compiled inside msvcr80.dll has a sizeof==64 (asssembly code multiplies by 64 when doing pointer arithmetic); in Debug mode, sizeof==56. in posixmodule.c, _PyVerify_fd() uses a sizeof of 56... It appears that Windows 2000 picks the first msvcr80.dll it finds on the PATH. So I played with copying various versions of it in the target directory. Here are the results, as reported by Visual Studio in the "Modules" pane. fails: C:\WINNT\system32\msvcr80.dll 8.00.50727.1433 fails: C:\python\trunk\PC\VS8.0\msvcr80.dll 8.00.50727.1433 works: C:\python\trunk\PC\VS8.0\msvcr80.dll 8.00.50727.762 fails: C:\python\trunk\PC\VS8.0\msvcr80.dll 8.00.50727.163 fails: C:\python\trunk\PC\VS8.0\msvcr80.dll 8.00.50727.42 works: C:\WINNT\system32\msvcr80d.dll 8.00.50727.762 DLL hell... The manifest embedded inside python27.dll contains version="8.0.50727.762", which is the only working version. So the problem will likely not happen on Windows XP, which enforces manifests. Is there a way to enforce the manifest information on Windows 2000 as well? If not, there may be several solutions: - disable the _PyVerify_fd() stuff on Windows 2000. - write clever code to detect the real sizeof(ioinfo) (for example: _get_osfhandle(1) returns __pioinfo[0][1]->osfhnd, which is a file opened for writing) |
|
|
| msg84848 - (view) |
Author: Kristján Valur Jónsson (kristjan.jonsson) * |
Date: 2009-03-31 18:03 |
| I already cope with different sizes of the debug and release (line 385): ifndef _DEBUG /* padding hack. 8 byte extra length observed at * runtime, for 32 and 64 bits when not in _DEBUG */ __int32 _padding[2]; #endif This fixed a problem as reported in another defect. But now you are saying that the struct size in Release is different for minor revisions of the dll? Oh dear. I wonder if we can just try both sizes... But it might cause a segfault. I had already assumed that the observed size difference was because the CRT sources were not in line with the actual minor version of the CRT. Obviously, this means that there are issues involved. We could also simply redistribute the MSVCRT in python/bin, as we are allowed to. This is how we solved similar issues when releasing EVE for various platforms. |
|
|
| msg84868 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2009-03-31 19:10 |
| > We could also simply redistribute the MSVCRT in python/bin, as we are > allowed to. We *do* distribute the CRT with Python, and it works just fine. The report is about VS 2005 |
|
|
| msg84898 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2009-03-31 20:59 |
| The problem with Windows 2000 is that it is not enough to distribute the CRT. For example, I am sure that a few months ago the version ending with .1433 was not present on my machine; the version in the system32 directory was .762 (the one shipped with vs2005 sp1). Trying several sizes is a good idea IMO, and is not likely to segfault if we only try with a low fd (1 or 2). I'll try to come with a patch. |
|
|
| msg84926 - (view) |
Author: Kristján Valur Jónsson (kristjan.jonsson) * |
Date: 2009-03-31 21:46 |
| When I meant "redistribute" I meant not install the CRT but keep it in the same dir as python2x.dll. This is one of the permitted redistro options of the MS crt. Of course, the point is moot since this is about the 2005 compile, not a C install. This hack is fast becoming quite troublesome :) I wonder if there is a way to glean the minor version number of the CRT from some exposed variable, rather than just trying out different sizes? |
|
|
| msg85005 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2009-04-01 13:35 |
| I found a correction using the _msize() function, which returns the size of a malloc'ed block. It is used to compute sizeof(ioinfo) at runtime. The exact definition of the structure is no longer important; only the first two fields are needed, which is a good thing IMO. Now the code seems really independent from changes in the CRT. Patch is attached. The test (info->osfhnd == _get_osfhandle(fd)) is not really needed, I left it outside an assert() because I wanted to test it on release builds. Tested (only) with VS2005, sizeof_ioinfo is correctly computed when presented to different versions of MSVCR80.dll. |
|
|
| msg85011 - (view) |
Author: Kristján Valur Jónsson (kristjan.jonsson) * |
Date: 2009-04-01 14:00 |
| Wow, clever use of _msize(), I hadn't thought of that. This is a much cleaner approach then the original and likely to be more robust. There is one problem though: Under VS2008 (and probably 2005), _get_osfhandle(fd) on an invalid fd will cause the assert that we are trying to avoid! Please do this only if you deem the fd to be "valid". If you fix this, it has my vote. |
|
|
| msg85018 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2009-04-01 15:21 |
| Oops, that's right. This second patch removes the check with _get_osfhandle(). It was not really necessary: when the trick does not work, python will tell it loudly enough. |
|
|
| msg85023 - (view) |
Author: Kristján Valur Jónsson (kristjan.jonsson) * |
Date: 2009-04-01 15:44 |
| Super, go ahead and check it in! |
|
|
| msg85028 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2009-04-01 15:59 |
| Sorry, but for the moment the only internet access I have is behind a firewall and I cannot use SVN. Could you do it instead? |
|
|
| msg85030 - (view) |
Author: Kristján Valur Jónsson (kristjan.jonsson) * |
Date: 2009-04-01 16:08 |
| Committed to trunk in revision: 70958 |
|
|
| msg85873 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2009-04-11 20:52 |
| Could someone port this to 3.1, please? I'm not Windows savy enough to do it. |
|
|
| msg85940 - (view) |
Author: Kristján Valur Jónsson (kristjan.jonsson) * |
Date: 2009-04-13 10:16 |
| Merged in 71559 |
|
|
| msg89137 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2009-06-09 07:20 |
| Why is this still open? |
|
|
| msg89140 - (view) |
Author: Kristján Valur Jónsson (kristjan.jonsson) * |
Date: 2009-06-09 08:36 |
| Closing this defect, as the issues seems worked around on our end. Btw, here is the issue on Microsoft's end. They've marked it as "won't fix". https://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx? FeedbackID=409955&wa=wsignin1.0 |
|
|