[Bug 36937] New: WinHttpReceiveRequest goes into infinite blocking on 304 responses
http://bugs.winehq.org/show_bug.cgi?id=36937 Bug ID: 36937 Summary: WinHttpReceiveRequest goes into infinite blocking on 304 responses Product: Wine Version: 1.7.22 Hardware: x86 OS: Linux Status: UNCONFIRMED Severity: major Priority: P2 Component: winhttp Assignee: wine-bugs(a)winehq.org Reporter: anduchs(a)gmail.com Created attachment 49045 --> http://bugs.winehq.org/attachment.cgi?id=49045 TestCase for the issue When requesting a website with an "If-Modified-Since"-header and the website returns "304 Not Modified" then the call to WinHttpReceiveResponse() blocks forever. The reason is that a 304-response does not have a Content-Length reader. For some reason it gets set to 4096 and winhttp tries to read this many bytes from the network even though nothing is going to come. This bug affects the "Marvel Heroes 2015 Launcher" during update checking. (Unfortunately game client still has some more issues) Attach are a Testing-Program and a patch that fixes it. (for the patch, I don't know if this would be the correct way to fix the issue, but it looked like a good place to put the fix) -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=36937 --- Comment #1 from Andreas <anduchs(a)gmail.com> --- Created attachment 49046 --> http://bugs.winehq.org/attachment.cgi?id=49046 Patch to fix the content-length on status 304 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=36937 Andreas <anduchs(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|1.7.22 |1.7.23 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=36937 Andreas <anduchs(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |download, patch, testcase -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=36937 --- Comment #2 from Hans Leidekker <hans(a)meelstraat.net> --- (In reply to Andreas from comment #0)
Created attachment 49045 [details] TestCase for the issue
Can you please integrate this test case in Wine's test suite and submit it to wine-patches? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=36937 --- Comment #3 from Andreas <anduchs(a)gmail.com> --- (In reply to Hans Leidekker from comment #2)
(In reply to Andreas from comment #0)
Created attachment 49045 [details] TestCase for the issue
Can you please integrate this test case in Wine's test suite and submit it to wine-patches?
I have hardly any knowledge on how wine and esp. its test-suit work... I'd be happy to be tutored, if you don't mind... :-) First things first: - How do you test infinite-loop-errors without killing the test-suit ? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=36937 --- Comment #4 from Bruno Jesus <00cpxxx(a)gmail.com> --- (In reply to Andreas from comment #3)
First things first: - How do you test infinite-loop-errors without killing the test-suit ?
You don't let it happen by sending the test patch together with the fix. Maybe this patch will help you see how to add a new test to the winhttp test suite: http://source.winehq.org/git/wine.git/commitdiff/1697eb6ee227b23d097b53ee01a... -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=36937 --- Comment #5 from Andreas <anduchs(a)gmail.com> --- Created attachment 49223 --> http://bugs.winehq.org/attachment.cgi?id=49223 Wine-TestSuit: Added test for winhttp 304 not modified responses This test-case is a little weird..... ;-) Since the connection to test.winehq.org times out after 5 seconds, winhttp will afterwards continue regularly... So the only way to test this is that the response takes less than these 5 seconds... Note: In the original program (Marvel Heroes), the application timeout was shorter than the server timeout which led to an error case. I don't know how to reconstruct this here though... :-( -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=36937 --- Comment #6 from Andreas <anduchs(a)gmail.com> --- Allmost forgot, could someone review this and give an OK if this kind of testcase is allowed ? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=36937 --- Comment #7 from Bruno Jesus <00cpxxx(a)gmail.com> --- (In reply to Andreas from comment #5)
Created attachment 49223 [details] Since the connection to test.winehq.org times out after 5 seconds, winhttp will afterwards continue regularly... So the only way to test this is that the response takes less than these 5 seconds...
That's why we have copies of HTTP headers [1] in the test and use a local socket server to test. If you see the test example I gave you I'm introducing a new HTTP responder header and adding it to be accessed through the server [2], if you do the same there won't be a need to connect to an external server. [1] http://source.winehq.org/git/wine.git/blob/HEAD:/dlls/winhttp/tests/winhttp.... [2] http://source.winehq.org/git/wine.git/blob/HEAD:/dlls/winhttp/tests/winhttp.... // comments are not allowed in C, use /* */ instead. The test cannot depend on timing IMO. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=36937 --- Comment #8 from Andreas <anduchs(a)gmail.com> --- Created attachment 49273 --> http://bugs.winehq.org/attachment.cgi?id=49273 NON-WORKING: Test for 304-responses Ok, I think this is what you were asking for. However, testing it against original wine reveiled that it does not trigger the error (which would have been a timeout in winhttp or similar), because the test-server does a closeSocket() right after sending the answer. So no error-condition in winhttp on this server... Got any idea, how to solve this ? Should I spawn a new thread per accepted connection ? Might also reveil errors in other tests as well... -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=36937 --- Comment #9 from Bruno Jesus <00cpxxx(a)gmail.com> --- What is the expected output when you run your test from comment 0? I modified the date to now when I run the test and this is the output in XP: WinHttpOpen: 11354112 WinHttpConnect: 11354368 WinHttpOpenRequest: 12058624 WinHttpSendRequest: 1:0 WinHttpReceiveResponse: 1:0 WinHttpQueryHeaders-size: 0:7a WinHttpQueryHeaders-data: 1:7a Header contents: H Done! This is in wine 1.7.24: WinHttpOpen: 1 WinHttpConnect: 2 WinHttpOpenRequest: 3 WinHttpSendRequest: 1:0 WinHttpReceiveResponse: 1:2f76 <<=== error 12150 (ERROR_WINHTTP_HEADER_NOT_FOUND) WinHttpQueryHeaders-size: 0:7a WinHttpQueryHeaders-data: 1:7a Header contents: H Done! -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=36937 --- Comment #10 from Andreas <anduchs(a)gmail.com> --- (In reply to Bruno Jesus from comment #9)
WinHttpOpen: 1 WinHttpConnect: 2 WinHttpOpenRequest: 3 WinHttpSendRequest: 1:0 WinHttpReceiveResponse: 1:2f76 <<=== error 12150 (ERROR_WINHTTP_HEADER_NOT_FOUND) WinHttpQueryHeaders-size: 0:7a WinHttpQueryHeaders-data: 1:7a Header contents: H Done!
Actually, that's not an error, since the return of WinHttpReceiveResponse is TRUE, the part after the : is just a leftover from winhttp internally. The Bug is triggered, if ReceiveResponse takes 5 Seconds: In this case, winhttp hangs until the server timesout the connection and closes it. The Bug is not present, if ReceiveResponse returns immediately. (P.S. if a 404 is returned, then it also returns immediately in all cases. I should probably have checked this) The effect on MarvelHeroes 2015 is the following: The server's timeout is 15 Seconds and the game hangs this long. After the timeout, the game does not like the response anymore, or the server does not or something like this and the patcher does not continue... I'm wondering if one could trigger a FALSE and LastError() if winhttp gets a timeout shorter than the server's timeout... I hope, this could clear things up... -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=36937 --- Comment #11 from Bruno Jesus <00cpxxx(a)gmail.com> --- Created attachment 49274 --> http://bugs.winehq.org/attachment.cgi?id=49274 test patch -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=36937 Bruno Jesus <00cpxxx(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |NEW Ever confirmed|0 |1 --- Comment #12 from Bruno Jesus <00cpxxx(a)gmail.com> --- I can confirm that it hangs until the connection is closed while on Windows it returns instantly. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=36937 Andreas <anduchs(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #49273|0 |1 is obsolete| | --- Comment #13 from Andreas <anduchs(a)gmail.com> --- Created attachment 49275 --> http://bugs.winehq.org/attachment.cgi?id=49275 Test for 304-responses, based on localhost-connection, but also on timing How do you like this compromise ? Still based on timing, but using the localhost-connection. But without sleep() or multi-threading... Does that work for you ? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=36937 Mike Ellery <mellery(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mellery(a)gmail.com -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=36937 Sebastian Lackner <sebastian(a)fds-team.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |sebastian(a)fds-team.de -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=36937 --- Comment #14 from Hans Leidekker <hans(a)meelstraat.net> --- This may be fixed by 2087a9fbe9ab5282fca8a55575fbc0126c3ac7bf. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=36937 Bruno Jesus <00cpxxx(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |2087a9fbe9ab5282fca8a55575f | |bc0126c3ac7bf Status|NEW |RESOLVED Resolution|--- |FIXED Severity|major |normal --- Comment #15 from Bruno Jesus <00cpxxx(a)gmail.com> --- (In reply to Hans Leidekker from comment #14)
This may be fixed by 2087a9fbe9ab5282fca8a55575fbc0126c3ac7bf.
The original issue in this bug was fixed, thanks Hans and Andreas. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=36937 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #16 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 1.7.34. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (1)
-
wine-bugs@winehq.org