classification
Title: test_fdopen fails with vs2005, release build on Windows 2000
Type: behavior Stage:
Components: Windows Versions: Python 3.1, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: amaury.forgeotdarc, benjamin.peterson, kristjan.jonsson, loewis, mhammond
Priority: critical Keywords: patch

Created on 2009-03-31 15:24 by amaury.forgeotdarc, last changed 2009-06-09 08:36 by kristjan.jonsson. This issue is now closed.

Files
File name Uploaded Description Edit
verify_fd.patch amaury.forgeotdarc, 2009-04-01 13:35
verify_fd-2.patch amaury.forgeotdarc, 2009-04-01 15:21
Messages (15)
msg84798 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) 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 "<stdin>", line 1, in <module>
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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2009-04-01 15:44
Super, go ahead and check it in!
msg85028 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) 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) * (Python committer) Date: 2009-04-01 16:08
Committed to trunk in revision: 70958
msg85873 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) 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) * (Python committer) Date: 2009-04-13 10:16
Merged in 71559
msg89137 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2009-06-09 07:20
Why is this still open?
msg89140 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) 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
History
Date User Action Args
2009-06-09 08:36:50kristjan.jonssonsetstatus: open -> closed
resolution: fixed
messages: + msg89140
2009-06-09 07:20:36loewissetmessages: + msg89137
2009-04-13 10:16:35kristjan.jonssonsetmessages: + msg85940
2009-04-11 20:52:12benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg85873
2009-04-01 16:08:55kristjan.jonssonsetmessages: + msg85030
2009-04-01 15:59:31amaury.forgeotdarcsetmessages: + msg85028
2009-04-01 15:44:41kristjan.jonssonsetmessages: + msg85023
2009-04-01 15:21:30amaury.forgeotdarcsetfiles: + verify_fd-2.patch

messages: + msg85018
2009-04-01 14:00:34kristjan.jonssonsetmessages: + msg85011
2009-04-01 13:35:53amaury.forgeotdarcsetfiles: + verify_fd.patch
keywords: + patch
messages: + msg85005
2009-03-31 21:46:11kristjan.jonssonsetmessages: + msg84926
2009-03-31 20:59:35amaury.forgeotdarcsetmessages: + msg84898
2009-03-31 19:10:47loewissetmessages: + msg84868
title: test_fdopen fails with vs2005, release build on Windows 2000 -> test_fdopen fails with vs2005, release build on Windows 2000
2009-03-31 18:03:41kristjan.jonssonsetmessages: + msg84848
2009-03-31 16:15:55amaury.forgeotdarcsetnosy: + kristjan.jonsson
2009-03-31 15:24:41amaury.forgeotdarccreate