[PATCH v2] urlmon: Always report available data before stop_binding.

[Jacek Caban]

On 1/5/22 08:22, Rémi Bernon wrote:
> On 1/4/22 23:23, Jacek Caban wrote:
>> Hi Rémi,
>>
>> On 1/4/22 21:31, Rémi Bernon wrote:
>>> The HTML launcher of several Rebellion games (all probably based on the
>>> same code) use a custom protocol handler which calls ReportResult in
>>> LockRequest, causing an invalid memory access when OnDataAvailable
>>> callback is called, as the binding has already been stopped and
>>> terminated.
>>>
>>> Wine-Bug:https://bugs.winehq.org/show_bug.cgi?id=46213
>>> Wine-Bug:https://bugs.winehq.org/show_bug.cgi?id=52286
>>> Signed-off-by: Rémi Bernon<rbernon at codeweavers.com>
>>> ---
>>>
>>> v2: Use an even more ad-hoc flag to detect LockRequest calls during
>>>      ReportData.
>>>
>>>   dlls/mshtml/tests/htmldoc.c | 18 +++++++++++++++++-
>>>   dlls/urlmon/binding.c       | 23 +++++++++++++++++++++++
>>>   2 files changed, 40 insertions(+), 1 deletion(-)
>>
>>
>> Those tests in htmldoc.c are a few layers above the problem, so they 
>> are not very informative. I did a quick experiment (see the 
>> attachment) with more precise test and an ad-hoc fix. It still needs 
>> better understanding, but does it help in your case?
>>
>>
>> Thanks,
>>
>> Jacek
>
> Hi Jacek,
>
> I must admit that I have very little idea how all this urlmon thing 
> works or does. Your patch only partially fix the launcher issue (it 
> doesn't crash anymore but it still doesn't show any window).
>
> As far as I understand, the custom protocol handler is:
>
> 1) Calling ReportResult within LockRequest.
> 2) Expecting OnDataAvailable to be called after.
> 3) Expecting OnStopBinding to be called after.
>
> I believe your patch doesn't do 3)? I'm not sure it expects this to 
> happen while ReportData is called, but it's how it usually happens, 
> and it definitely expect OnStopBinding to be called after ReportResult 
> has been, to continue.


Sure, but I suspect that it should be handled one layer bellow what your 
patch does. bindprot.c is between Binding object and protocol itself and 
it already has some tools to do deal with such cases. For example, the 
attached patch fixes Zombie Army Trilogy and is close to what I'd 
expect. I'd like to do some more tests to understand how exactly it is 
supposed to work.


>
> It also causes ReportResult to return E_FAIL when called within 
> LockRequest, which is not what native does (at least it's not with the 
> mshtml version of the test). 


That matched what my test showed, but it seems that it fails on some 
Windows, so there was probably a problem with the test itself.


Thanks,

Jacek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.diff
Type: text/x-patch
Size: 803 bytes
Desc: not available
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20220105/bb64007b/attachment.bin>