According to MSDN, the hEvent member of an OVERLAPPED structure can be NULL:
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dllproc/bas... OVERLAPPED ... Members ... hEvent Handle to an event set to the signaled state when the operation has been completed. The calling process must set this member either to zero or a valid event handle before calling any overlapped functions.
A program I'm trying to run takes advantage of this, and as a result triggered a lot of these errors from Wine:
err:file:GetOverlappedResult lpOverlapped->hEvent was null
I wrote a patch that solves this, although it may not be the best solution.
If when ReadFile is called overlapped->hEvent is NULL, the patch generates a new event with CreateEventA and sets the structure member to that event. To create a unique name, it uses the hex address of the overlapped structure along with a fairly long string, which is pretty likely to be unique.
This works for the application I'm running. If this seems like a good enough solution, I can easily modify the other functions which initiate an overlapped read to do the same.
----ScottG.
Scott W Gifford a écrit :
According to MSDN, the hEvent member of an OVERLAPPED structure can be NULL:
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dllproc/bas... OVERLAPPED ... Members ... hEvent Handle to an event set to the signaled state when the operation has been completed. The calling process must set this member either to zero or a valid event handle before calling any overlapped functions.
A program I'm trying to run takes advantage of this, and as a result triggered a lot of these errors from Wine:
err:file:GetOverlappedResult lpOverlapped->hEvent was nullI wrote a patch that solves this, although it may not be the best solution.
If when ReadFile is called overlapped->hEvent is NULL, the patch generates a new event with CreateEventA and sets the structure member to that event. To create a unique name, it uses the hex address of the overlapped structure along with a fairly long string, which is pretty likely to be unique.
This works for the application I'm running. If this seems like a good enough solution, I can easily modify the other functions which initiate an overlapped read to do the same.
I'm not sure this is a correct fix: - you modify the passed overlapped structure (by filling in the hEvent), I'm not sure Windows does it this way - the low level functions (in ntdll) already support the case where hEvent is NULL (and recreate an internal event for the wait operation)
I'd rather say it's GetOverlappedResult which is to blame. If the hEvent is NULL, then we shouldn't return any error (as we do) but rather assume all wait operation succeed (if hEven is NULL, ReadFile and WriteFile operations are synchronous, so calling GetOverlappedResult is a no-op). But, this should be verified by small tests (and added to wine's test suite).
PS: some comments on your patch (not so useless regarding what I wrote above): - CreateEvent can take a NULL parameter (for the name) which would solve the name construction issue - you don't need to but the hEvent as a static variable
A+
I wrote a simple test program and ran it under Windows, to see what Windows does. I've attached the program here. I found that...
Eric Pouech pouech-eric@wanadoo.fr writes:
[...]
I'm not sure this is a correct fix:
- you modify the passed overlapped structure (by filling in the
hEvent), I'm not sure Windows does it this way
...Windows doesn't do it this way, although according to the docs other members of the overlapped structure are modified by Windows.
Unfortunately, I'm not sure of another straightforward way to implement this. Unless somebody can come up with one, I think it's better to have Wine's behavior deviate slightly from Windows than to completely fail to support this feature.
[...]
I'd rather say it's GetOverlappedResult which is to blame. If the hEvent is NULL, then we shouldn't return any error (as we do) but rather assume all wait operation succeed (if hEven is NULL, ReadFile and WriteFile operations are synchronous, so calling GetOverlappedResult is a no-op).
This turns out not to be the case. As long as lpOverlapped is supplied, a NULL hEvent will work fine and allows overlapped reads.
But, this should be verified by small tests (and added to wine's test suite).
I agree; consider the attached file the first stab at a test program for this feature.
PS: some comments on your patch (not so useless regarding what I wrote above):
- CreateEvent can take a NULL parameter (for the name) which would
solve the name construction issue
Ah, I didn't know that. If we keep calling CreateEvent over and over again with a NULL parameter, will we eventually run out of some resource? If so, it may still be best to construct a name, since calling CreateEvent with the same name twice will apparently avoid creating another event.
- you don't need to but the hEvent as a static variable
Thanks, that's a relic from an earlier version. If the other aspects of this patch are OK, I'll fix it in an update.
The attached test program uses overlapped reads with a NULL hEvent in the OVERLAPPED structure to connect to an IP address and port given on the command line. Here's an example from Windows:
C:>overlapped-test 216.12.213.139 25 Calling ReadFile ReadFile returned ERROR_IO_PENDING, as expected ov .hEvent=00000000 Calling Sleep(2000) Calling GetOverlappedResult GetOverlappedresult read 85 bytes: 220 2search2.com ESMTP Sendmail 8.10.2-SOL3/8.10.2; Mon, 17 May 2004 11:58:26 -0400
----ScottG.
...Windows doesn't do it this way, although according to the docs other members of the overlapped structure are modified by Windows.
Ok, we need to rewrite GetOverlappedResult to use the internal structures (and get the hEvent we might have created in ReadFile), instead of relying on the one from the overlapped struct itself
I'd rather say it's GetOverlappedResult which is to blame. If the hEvent is NULL, then we shouldn't return any error (as we do) but rather assume all wait operation succeed (if hEven is NULL, ReadFile and WriteFile operations are synchronous, so calling GetOverlappedResult is a no-op).
This turns out not to be the case. As long as lpOverlapped is supplied, a NULL hEvent will work fine and allows overlapped reads.
Could you also test it with opening a file (files and sockets semantics are somehow different)
Ah, I didn't know that. If we keep calling CreateEvent over and over again with a NULL parameter, will we eventually run out of some resource? If so, it may still be best to construct a name, since calling CreateEvent with the same name twice will apparently avoid creating another event.
if you run out of resource with calling with a NULL parameter, you'll also get out of resource with constructing the name, so you don't need to bother here
The attached test program uses overlapped reads with a NULL hEvent in the OVERLAPPED structure to connect to an IP address and port given on the command line. Here's an example from Windows:
C:>overlapped-test 216.12.213.139 25 Calling ReadFile ReadFile returned ERROR_IO_PENDING, as expected ov .hEvent=00000000 Calling Sleep(2000) Calling GetOverlappedResult GetOverlappedresult read 85 bytes: 220 2search2.com ESMTP Sendmail 8.10.2-SOL3/8.10.2; Mon, 17 May 2004 11:58:26 -0400
just for fun, what gives without the Sleep ? A+
Eric Pouech a écrit :
...Windows doesn't do it this way, although according to the docs other members of the overlapped structure are modified by Windows.
Ok, we need to rewrite GetOverlappedResult to use the internal structures (and get the hEvent we might have created in ReadFile), instead of relying on the one from the overlapped struct itself
Does the attached patch help ? A+
Index: dlls/kernel/file.c =================================================================== RCS file: /home/cvs/cvsroot/wine/wine/dlls/kernel/file.c,v retrieving revision 1.19 diff -u -r1.19 file.c --- dlls/kernel/file.c 13 May 2004 20:21:25 -0000 1.19 +++ dlls/kernel/file.c 17 May 2004 17:13:44 -0000 @@ -492,44 +492,44 @@ BOOL WINAPI GetOverlappedResult(HANDLE hFile, LPOVERLAPPED lpOverlapped, LPDWORD lpTransferred, BOOL bWait) { - DWORD r; + DWORD r = WAIT_OBJECT_0;
TRACE("(%p %p %p %x)\n", hFile, lpOverlapped, lpTransferred, bWait);
- if (lpOverlapped==NULL) + if (lpOverlapped == NULL) { ERR("lpOverlapped was null\n"); return FALSE; } - if (!lpOverlapped->hEvent) - { - ERR("lpOverlapped->hEvent was null\n"); - return FALSE; - } - if ( bWait ) { - do { + do + { TRACE("waiting on %p\n",lpOverlapped); r = WaitForSingleObjectEx(lpOverlapped->hEvent, INFINITE, TRUE); TRACE("wait on %p returned %ld\n",lpOverlapped,r); - } while (r==STATUS_USER_APC); + } while (r == WAIT_IO_COMPLETION); } else if ( lpOverlapped->Internal == STATUS_PENDING ) { /* Wait in order to give APCs a chance to run. */ /* This is cheating, so we must set the event again in case of success - it may be a non-manual reset event. */ - do { + do + { TRACE("waiting on %p\n",lpOverlapped); r = WaitForSingleObjectEx(lpOverlapped->hEvent, 0, TRUE); TRACE("wait on %p returned %ld\n",lpOverlapped,r); - } while (r==STATUS_USER_APC); - if ( r == WAIT_OBJECT_0 ) - NtSetEvent ( lpOverlapped->hEvent, NULL ); + } while (r == WAIT_IO_COMPLETION); + if ( r == WAIT_OBJECT_0 && lpOverlapped->hEvent ) + NtSetEvent( lpOverlapped->hEvent, NULL ); } - - if(lpTransferred) + if (r == WAIT_FAILED && lpOverlapped->hEvent) + { + ERR("wait operation failed\n"); + return FALSE; + } + if (lpTransferred) *lpTransferred = lpOverlapped->InternalHigh;
switch ( lpOverlapped->Internal ) @@ -537,11 +537,11 @@ case STATUS_SUCCESS: return TRUE; case STATUS_PENDING: - SetLastError ( ERROR_IO_INCOMPLETE ); - if ( bWait ) ERR ("PENDING status after waiting!\n"); + SetLastError( ERROR_IO_INCOMPLETE ); + if ( bWait ) ERR("PENDING status after waiting!\n"); return FALSE; default: - SetLastError ( RtlNtStatusToDosError ( lpOverlapped->Internal ) ); + SetLastError( RtlNtStatusToDosError( lpOverlapped->Internal ) ); return FALSE; } } @@ -635,7 +635,6 @@ return (HFILE)create_file_OF( path, mode & ~OF_CREATE ); }
- /*********************************************************************** * _lread (KERNEL32.@) */
Eric Pouech pouech-eric@wanadoo.fr writes:
[...]
This turns out not to be the case. As long as lpOverlapped is supplied, a NULL hEvent will work fine and allows overlapped reads.
Could you also test it with opening a file (files and sockets semantics are somehow different)
I did, and it just returned the entire block in the ReadFile. I don't know how to open a file in a way that the ReadFile will not return immediately. The source code for this is a straightforward change to my earlier version; let me know if you'd like a copy.
[...]
The attached test program uses overlapped reads with a NULL hEvent in the OVERLAPPED structure to connect to an IP address and port given on the command line. Here's an example from Windows: C:>overlapped-test 216.12.213.139 25 Calling ReadFile ReadFile returned ERROR_IO_PENDING, as expected ov .hEvent=00000000 Calling Sleep(2000) Calling GetOverlappedResult GetOverlappedresult read 85 bytes: 220 2search2.com ESMTP Sendmail 8.10.2-SOL3/8.10.2; Mon, 17 May 2004 11:58:26 -0400
just for fun, what gives without the Sleep ?
Same results.
Eric Pouech pouech-eric@wanadoo.fr writes:
Eric Pouech a écrit :
...Windows doesn't do it this way, although according to the docs other members of the overlapped structure are modified by Windows.
Ok, we need to rewrite GetOverlappedResult to use the internal structures (and get the hEvent we might have created in ReadFile), instead of relying on the one from the overlapped struct itself
Does the attached patch help ?
Yes, it solves the problem completely for me. I have to admit I don't understand why it works, but thanks!! :)
---ScottG.
-
Eric Pouech -
Scott W Gifford