Hi Markus, Wine development is driven by test cases checked in to the Wine source tree. Please extend the test cases in dlls/oleaut32/tests to verify that oleaut32 is caching BSTRs properly, and make sure that test passes on Windows and is marked todo_wine. Once we have a test case like that checked into the Wine source tree, we'll be in better shape to accept a patch that fixes bug 12460. Thanks!
Am Freitag, den 09.10.2009, 00:27 -0700 schrieb Dan Kegel:
Hi Markus, Wine development is driven by test cases checked in to the Wine source tree. Please extend the test cases in dlls/oleaut32/tests to verify that oleaut32 is caching BSTRs properly, and make sure that test passes on Windows and is marked todo_wine. Once we have a test case like that checked into the Wine source tree, we'll be in better shape to accept a patch that fixes bug 12460. Thanks!
Already sent. Take a look here: http://www.winehq.org/pipermail/wine-devel/2009-October/078957.html So I will ask again with this mail if someone can verify if the test above succeeds in various Windows versions. Just to make sure that it only fails in Wine. If I get positive feedback I will send it do wine-patches. Thanks in advance. Markus
On 10/09/2009 09:38 AM, Markus Stockhausen wrote:
Am Freitag, den 09.10.2009, 00:27 -0700 schrieb Dan Kegel:
Hi Markus, Wine development is driven by test cases checked in to the Wine source tree. Please extend the test cases in dlls/oleaut32/tests to verify that oleaut32 is caching BSTRs properly, and make sure that test passes on Windows and is marked todo_wine. Once we have a test case like that checked into the Wine source tree, we'll be in better shape to accept a patch that fixes bug 12460. Thanks!
Already sent. Take a look here: http://www.winehq.org/pipermail/wine-devel/2009-October/078957.html
So I will ask again with this mail if someone can verify if the test above succeeds in various Windows versions. Just to make sure that it only fails in Wine. If I get positive feedback I will send it do wine-patches.
Thanks in advance.
Markus
Hi Markus, Test results: Win98SE : no failures WinME : no failures NT4 : no failures W2K : no failures XP Prof : no failures W2K3 : no failures Vista Ult SP2 : no failures -- Cheers, Paul.
Am Freitag, den 09.10.2009, 09:56 +0200 schrieb Paul Vriens:
On 10/09/2009 09:38 AM, Markus Stockhausen wrote:
Am Freitag, den 09.10.2009, 00:27 -0700 schrieb Dan Kegel:
Hi Markus, Wine development is driven by test cases checked in to the Wine source tree. Please extend the test cases in dlls/oleaut32/tests to verify that oleaut32 is caching BSTRs properly, and make sure that test passes on Windows and is marked todo_wine. Once we have a test case like that checked into the Wine source tree, we'll be in better shape to accept a patch that fixes bug 12460. Thanks!
Already sent. Take a look here: http://www.winehq.org/pipermail/wine-devel/2009-October/078957.html
So I will ask again with this mail if someone can verify if the test above succeeds in various Windows versions. Just to make sure that it only fails in Wine. If I get positive feedback I will send it do wine-patches.
Thanks in advance.
Markus
Hi Markus,
Test results:
Win98SE : no failures WinME : no failures NT4 : no failures W2K : no failures XP Prof : no failures W2K3 : no failures Vista Ult SP2 : no failures
Fine, thanks for that. I will sent a final version of the testcase.
Markus Stockhausen <markus.stockhausen(a)collogia.de> wrote:
Please extend the test cases in dlls/oleaut32/tests to verify that oleaut32 is caching BSTRs properly, and make sure that test passes on Windows and is marked todo_wine.
Already sent. Take a look here: http://www.winehq.org/pipermail/wine-devel/2009-October/078957.html
Three problems: You didn't mark the test todo_wine. The name of the test is cryptic, don't you want to call it something like test_bstr_caching()? This change should not be included in the patch: - test_VarImp(); + test_VarImp(); Other notes: You might want to also verify the contents of the string rather than the length, and do it right after the free as well. Also, how did you decide on the size of sz99?
So I will ask again with this mail if someone can verify if the test above succeeds in various Windows versions. Just to make sure that it only fails in Wine. If I get positive feedback I will send it do wine-patches.
Passes on vista. Have you tested it with 'winetricks dcom98'? Passing those two should suffice. - Dan
Am Freitag, den 09.10.2009, 01:05 -0700 schrieb Dan Kegel:
Also, how did you decide on the size of sz99?
I do not want to explain this in detail, as it is the result of looking at internal behaviour (something that is not liked very much as I understand now). But to make a long story short: This will ensure that oleaut will allocate new memory instead of reusing its caches. Markus
On 10/09/2009 10:12 AM, Markus Stockhausen wrote:
Am Freitag, den 09.10.2009, 01:05 -0700 schrieb Dan Kegel:
Also, how did you decide on the size of sz99?
I do not want to explain this in detail, as it is the result of looking at internal behaviour (something that is not liked very much as I understand now).
But to make a long story short: This will ensure that oleaut will allocate new memory instead of reusing its caches.
Markus
Hi Markus, Isn't there a way that you can change the tests to show this number (in some kind of loop by creating a larger second string on the go)? -- Cheers, Paul.
Am Freitag, den 09.10.2009, 13:43 +0200 schrieb Paul Vriens:
Hi Markus,
Isn't there a way that you can change the tests to show this number (in some kind of loop by creating a larger second string on the go)?
It simply boils down to this one and only testcase. A SysFreeString will always preserve the contents of the heap and will put the address into some caching freelist. It does not matter how these freelists are organized because of two reasons: - The application does not know the state of oleaut freelists. Earlier calls could have filled them up to some state. - This leads to the second situation that the application cannot anticipate. Will it get a new memory area or something from a freelist when doing a SysAllocString. Reading bugzilla i can see that errors due to missing BSTR caching are ancient WINE companions. But only because of faulty applications. In my opinion there are three ways to go: - close all associated bugs and blame the developers - take some weeks and try to explore the caching behaviour of oleaut - implement only that bit of caching that is required for the bugs My sent in testcase is the basis for the third solution and as my code proposal shows I'm willing to spend some time to get it fixed. Best regards. Markus
On 10/09/2009 02:07 PM, Markus Stockhausen wrote:
Am Freitag, den 09.10.2009, 13:43 +0200 schrieb Paul Vriens:
Hi Markus,
Isn't there a way that you can change the tests to show this number (in some kind of loop by creating a larger second string on the go)?
It simply boils down to this one and only testcase. A SysFreeString will always preserve the contents of the heap and will put the address into some caching freelist.
It does not matter how these freelists are organized because of two reasons:
- The application does not know the state of oleaut freelists. Earlier calls could have filled them up to some state. - This leads to the second situation that the application cannot anticipate. Will it get a new memory area or something from a freelist when doing a SysAllocString.
Reading bugzilla i can see that errors due to missing BSTR caching are ancient WINE companions. But only because of faulty applications. In my opinion there are three ways to go:
- close all associated bugs and blame the developers - take some weeks and try to explore the caching behaviour of oleaut - implement only that bit of caching that is required for the bugs
My sent in testcase is the basis for the third solution and as my code proposal shows I'm willing to spend some time to get it fixed.
Best regards.
Markus
Hi Markus, My question about a modified test case came from your statement: "I do not want to explain this in detail, as it is the result of looking at internal behaviour (something that is not liked very much as I understand now)." If that sz99 (or now sz128) came from "looking at internal behaviour", I'm not sure if that would raise some eyebrows. -- Cheers, Paul.
Am Freitag, den 09.10.2009, 14:15 +0200 schrieb Paul Vriens:
If that sz99 (or now sz128) came from "looking at internal behaviour", I'm not sure if that would raise some eyebrows.
As I said "looking at internal behaviour" are debugging messages in the IMalloc routines of ifs.c. Simply something you cannot catch when writing testcases only.
On Fri, Oct 9, 2009 at 2:25 PM, Markus Stockhausen <markus.stockhausen(a)collogia.de> wrote:
Am Freitag, den 09.10.2009, 14:15 +0200 schrieb Paul Vriens:
If that sz99 (or now sz128) came from "looking at internal behaviour", I'm not sure if that would raise some eyebrows.
As I said "looking at internal behaviour" are debugging messages in the IMalloc routines of ifs.c. Simply something you cannot catch when writing testcases only.
Blackbox reverse engineering is feeding test input to a function and looking at the output. A trace of calls made by the native MS dll while looking under Wine isn't blackbox. It is considered about the same as looking at disassembled Microsoft code. I'm not sure how to proceed from this though. Roderick
Roderick Colenbrander wrote:
On Fri, Oct 9, 2009 at 2:25 PM, Markus Stockhausen <markus.stockhausen(a)collogia.de> wrote:
Am Freitag, den 09.10.2009, 14:15 +0200 schrieb Paul Vriens:
If that sz99 (or now sz128) came from "looking at internal behaviour", I'm not sure if that would raise some eyebrows.
As I said "looking at internal behaviour" are debugging messages in the IMalloc routines of ifs.c. Simply something you cannot catch when writing testcases only.
Blackbox reverse engineering is feeding test input to a function and looking at the output. A trace of calls made by the native MS dll while looking under Wine isn't blackbox. It is considered about the same as looking at disassembled Microsoft code.
I'm not sure how to proceed from this though. He probably can still write the test case; I really doubt that Alexandre will accept a patch to fix the issue. Somebody else would have to do it based on Markus' description.
bye michael
Michael Stefaniuc <mstefani(a)redhat.com> writes:
Roderick Colenbrander wrote:
Blackbox reverse engineering is feeding test input to a function and looking at the output. A trace of calls made by the native MS dll while looking under Wine isn't blackbox. It is considered about the same as looking at disassembled Microsoft code.
I'm not sure how to proceed from this though. He probably can still write the test case; I really doubt that Alexandre will accept a patch to fix the issue. Somebody else would have to do it based on Markus' description.
If the test case is based on observing internal behavior that's not acceptable either. Even if someone else fixes it, the test would force the fixer to replicate internal details. -- Alexandre Julliard julliard(a)winehq.org
Am Freitag, den 09.10.2009, 15:44 +0200 schrieb Alexandre Julliard:
If the test case is based on observing internal behavior that's not acceptable either. Even if someone else fixes it, the test would force the fixer to replicate internal details.
Hi, I can see all of your concerns and the message of what not to do is understood. If my enthusiasm to reveal the reason for the error got me too far I'm sorry and this is my official excuse. I'll try to rewind any knowledge to the point where it all started. And that was here: The original goal of my journey was to understand why BSTR memory structures constantly survived a SysFree cycle in Windows while they got overwritten for some case in Wine. From what I read in bugzilla the argumentation always explained, that this comes from heap management. The tip to solve this with native oleaut, further links to MSDN documentation about SetOaNoCache() and the KB article http://support.microsoft.com/kb/937360 led the way that heap was not freed upon calling SysFreeString. With this bit of knowledge I ask if the following testcase will be valid. It should not care about any code inside the "black box": ... for (i=10;i<100;i++) { s1 = SysAllocStringLen(sz128,i); SysFreeString(s1); HeapAlloc(GetProcessHeap(),0,10); s2 = SysAllocStringLen(sz128,i); SysFreeString(s1); ok(s1==s2, "got a new memory address\n"); } ... The only development goal this testcase has, is to request a function that provides the same memory address in two subsequent Alloc/Dealloc calls with the same string size. Noone should be forced to implement something that goes anywhere close to native implementation. A start maybe.
Am Freitag, den 09.10.2009, 14:15 +0200 schrieb Paul Vriens:
If that sz99 (or now sz128) came from "looking at internal behaviour", I'm not sure if that would raise some eyebrows.
... and I simply allocate a "very large" string to ensure that no reuse of the just freed small memory portion can occur. Markus
On 10/09/2009 02:15 PM, Paul Vriens wrote:
On 10/09/2009 02:07 PM, Markus Stockhausen wrote:
Am Freitag, den 09.10.2009, 13:43 +0200 schrieb Paul Vriens:
Hi Markus,
Isn't there a way that you can change the tests to show this number (in some kind of loop by creating a larger second string on the go)?
It simply boils down to this one and only testcase. A SysFreeString will always preserve the contents of the heap and will put the address into some caching freelist.
It does not matter how these freelists are organized because of two reasons:
- The application does not know the state of oleaut freelists. Earlier calls could have filled them up to some state. - This leads to the second situation that the application cannot anticipate. Will it get a new memory area or something from a freelist when doing a SysAllocString.
Reading bugzilla i can see that errors due to missing BSTR caching are ancient WINE companions. But only because of faulty applications. In my opinion there are three ways to go:
- close all associated bugs and blame the developers - take some weeks and try to explore the caching behaviour of oleaut - implement only that bit of caching that is required for the bugs
My sent in testcase is the basis for the third solution and as my code proposal shows I'm willing to spend some time to get it fixed.
Best regards.
Markus
Hi Markus,
My question about a modified test case came from your statement:
"I do not want to explain this in detail, as it is the result of looking at internal behaviour (something that is not liked very much as I understand now)."
If that sz99 (or now sz128) came from "looking at internal behaviour", I'm not sure if that would raise some eyebrows.
Ok, what about something like the attached testcase? This one succeeds on W2K3 (didn't test on other boxes yet). Does it prove anything? -- Cheers, Paul.
participants (6)
-
Alexandre Julliard -
Dan Kegel -
Markus Stockhausen -
Michael Stefaniuc -
Paul Vriens -
Roderick Colenbrander