Hello All,
I wrote this some time ago:
Dr J A Gow wrote:
Hello All,
I have some regression problems relating to Wine in a commercial ECAD app 'Easy-PC' version 9.0, available from http://www.numberone.com
Since then I have been doing some more digging into the problem and have come up with a test that triggers the bugs: i.e. passes on Windows and fails on Wine (latest CVS). Looking at the failure mode I feel that it has the potential to break a _lot_ of apps.
It is as I thought that there is some issue with the object destructor for the storage object being called and not actually releasing the object. I have attached the complete patch to the tests for storage32 to this message if anyone could try it I would be grateful, but I will just describe some of the reasoning behind it first.
It seems that the problem is not the grfMode with which the file is opened, but a reference counting method used within storage32. A call to the destructor of the storage object is not destroying the object - with it left open the error appears when a subsequent call to open the storage object fails with access issues because the previous one is left open. So, I introduced this test that exactly mimics the behaviour of the app. To do this, I needed a storage object that contained a stream, so I wrote code in the appropriate conformance test module to create one:
r = StgOpenStorage( filename, NULL, STGM_TRANSACTED | STGM_WRITE | STGM_SHARE_DENY_WRITE, NULL, 0, &stg); ok(r==S_OK, "StgOpenStorage failed\n"); if(stg) { static const WCHAR stmname[] = { 'w','i','n','e','t','e','s','t',0}; IStream *stm = NULL;
r = IStorage_CreateStream( stg, stmname, STGM_WRITE | STGM_SHARE_EXCLUSIVE, 0, 0, &stm ); ok(r == S_OK, "CreateStream should succeed\n");
if(stm) { r=IStorage_Commit(stg,STGC_DEFAULT); ok(r==S_OK, "StorageCommit failed\n");
}
// Windows reference counting seems different....
r = IStorage_Release(stg); ok(r == 0, "wrong ref count"); printf("- ref count = %lx\n",r);
if(r) { r = IStorage_Release(stg); ok(r == 0, "wrong ref count"); printf(" - ref count = %lx\n",r); }
}
It was here where I got the surprise! I expected this code to work on both Wine and Windows, so the next part of the patch could trigger the bug. However I found that on Windows, the first call to IStorage_Release returns zero, and the storage object is destroyed. Under Wine however, the first call to IStorage_Release returns 1 but does not destroy the object - the second call to IStorage_Release returns zero and destroys the object. It appears that Windows takes no notice of reference counts when the IStorage destructor is called! I only discovered this because in the original test code I had two calls to IStorage_Release straight after each other without the if(r) test: and although it worked on Wine it segfaulted on Windows due to the 'stg' object being destroyed on the first call. This proved the difference in behaviour.
Now on to the second part of the test, that attempted to re-open the closed storage object, then open the stream within it:
/* Easy-PC test 2 - this looks for the OpenStream mode check. Fails under Wine */
r = StgOpenStorage( filename, NULL, 0x00010020, NULL, 0, &stg); ok(r==S_OK, "StgOpenStorage failed\n"); if(stg) { static const WCHAR stmname[] = { 'w','i','n','e','t','e','s','t',0}; IStream *stm = NULL;
r = IStorage_OpenStream( stg, stmname, 0, STGM_SHARE_EXCLUSIVE|STGM_READWRITE, 0, &stm ); ok(r == S_OK, "OpenStream should succeed - "); printf("rc from OpenStream : %lx\n",r);
r = IStorage_Release(stg); ok(r == 0, "wrong ref count\n"); }
Again, this works on Windows, but the OpenStream call fails with an access denied error on Wine. This is the exact mode of the bug in the app.
Could one of the ole32 developers please contact me: I am happy to work with them to resolve this problem as I need this app to work under Wine, but not being a Windows programmer I am not fully familiar with the storage API. In the meantime I will continue my investigation and see if I can fix this directly, but I am sure it will happen sooner if I could work with the ole32 developer.
Many thanks for listening. The patch is below, I will submit this for inclusion in CVS.
John.
Index: wine/dlls/ole32/tests/storage32.c =================================================================== RCS file: /home/wine/wine/dlls/ole32/tests/storage32.c,v retrieving revision 1.14 diff -u -p -r1.14 storage32.c --- wine/dlls/ole32/tests/storage32.c 6 Oct 2005 11:38:45 -0000 1.14 +++ wine/dlls/ole32/tests/storage32.c 8 Feb 2006 18:17:03 -0000 @@ -448,6 +448,77 @@ static void test_open_storage(void) ok(r == 0, "wrong ref count\n"); }
+ + + // + // EASY-PC TESTS + // + // There are two bugs related to Wine and Easy-PC. This pair of tests + // triggers the first of the two. + // + // These tests trigger the same bug in Wine that Easy-PC does. They: + // + // 1) Open a storage file, write a stream to it, commit it and then close + // the file. This is just so we have a storage file with a stream in + // it. This operation itself does not trigger the bug. + // + // 2) It then performs the action that triggers the bug. It opens the + // storage file with a grfMode of 0x00010020 + // (STGM_TRANSACTED|STGM_READ|STGM_SHARE_DENY_WRITE). It then opens + // a stream in the file with a grfMode of STGM_SHARE_EXCLUSIVE|STGM_READWRITE + // This test works under Windows, but fails under Wine. + + + /* Easy-PC bug test 1 - this Creates a stream within an object and should not fail*/ + + r = StgOpenStorage( filename, NULL, STGM_TRANSACTED | STGM_WRITE | STGM_SHARE_DENY_WRITE, NULL, 0, &stg); + ok(r==S_OK, "StgOpenStorage failed\n"); + if(stg) + { + static const WCHAR stmname[] = { 'w','i','n','e','t','e','s','t',0}; + IStream *stm = NULL; + + r = IStorage_CreateStream( stg, stmname, STGM_WRITE | STGM_SHARE_EXCLUSIVE, + 0, 0, &stm ); + ok(r == S_OK, "CreateStream should succeed\n"); + + if(stm) { + r=IStorage_Commit(stg,STGC_DEFAULT); + ok(r==S_OK, "StorageCommit failed\n"); + + } + + // Windows reference counting seems different.... + + r = IStorage_Release(stg); + ok(r == 0, "wrong ref count"); printf("- ref count = %lx\n",r); + + if(r) { + r = IStorage_Release(stg); + ok(r == 0, "wrong ref count"); printf(" - ref count = %lx\n",r); + } + + } + + /* Easy-PC test 2 - this looks for the OpenStream mode check. Fails under Wine */ + + r = StgOpenStorage( filename, NULL, 0x00010020, NULL, 0, &stg); + ok(r==S_OK, "StgOpenStorage failed\n"); + if(stg) + { + static const WCHAR stmname[] = { 'w','i','n','e','t','e','s','t',0}; + IStream *stm = NULL; + + r = IStorage_OpenStream( stg, stmname, 0, STGM_SHARE_EXCLUSIVE|STGM_READWRITE, 0, &stm ); + ok(r == S_OK, "OpenStream should succeed - "); printf("rc from OpenStream : %lx\n",r); + + r = IStorage_Release(stg); + ok(r == 0, "wrong ref count\n"); + } + + // + // End of Easy-PC bug test. + r = DeleteFileW(filename); ok(r, "file didn't exist\n"); }