https://bugs.winehq.org/show_bug.cgi?id=55952
Bug ID: 55952 Summary: Wine Mono HttpResponseStreamTest and HttpWebResponseTest fail Product: Wine Version: 8.21 Hardware: x86-64 OS: Linux Status: NEW Severity: normal Priority: P2 Component: wineserver Assignee: wine-bugs@winehq.org Reporter: madewokherd@gmail.com Distribution: ---
Regression from 8.20 to 8.21.
To reproduce, download the Wine Mono test suite from https://dl.winehq.org/wine/wine-mono/8.1.0/wine-mono-8.1.0-tests.zip and run: wine run-tests.exe MonoTests.System.Net.HttpResponseStreamTest MonoTests.System.Net.HttpWebResponseTest
In the good case I get 52 tests passed, 4 tests failed (no list of failing tests because these are "expected" failures).
In the bad case I get 2 tests passed and 54 failed, with a list of unexpected failures (I guess one of the tests in HttpWebResponseTest still works).
Bisect showed this:
a866d16ff8f737b864ceee10931aae06c51bed39 is the first bad commit commit a866d16ff8f737b864ceee10931aae06c51bed39 Author: Zebediah Figura zfigura@codeweavers.com Date: Fri Jul 15 20:03:02 2022 -0500
server: Return ERROR_CONNECTION_RESET when trying to recv() on a reset socket.
https://bugs.winehq.org/show_bug.cgi?id=55952
Esme Povirk madewokherd@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |download, regression, | |source, testcase CC| |madewokherd@gmail.com, | |z.figura12@gmail.com URL| |https://dl.winehq.org/wine/ | |wine-mono/8.1.0/wine-mono-8 | |.1.0-tests.zip Regression SHA1| |a866d16ff8f737b864ceee10931 | |aae06c51bed39
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #1 from Esme Povirk madewokherd@gmail.com --- Looks like I missed that this also broke MonoTests.System.Net.HttpWebRequestTest:NotModifiedSince
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #2 from Zeb Figura z.figura12@gmail.com --- As far as I can tell everything's working correctly on the Wine end? This looks like a mono bug to me.
Specifically, in SocketResponder.cs I see this:
// On Windows a Receive() is needed here before Shutdown() to consume the data some tests send. listenSocket.ReceiveTimeout = 10 * 1000; listenSocket.Receive (new byte [0]);
That is translated (and I think is supposed to be translated) into a zero-byte recv() call, which waits for the data but doesn't actually consume it.
Shutting down and exiting with data left in the pipe results in a TCP reset, both on Unix and on Windows, and that's propagated to the other end.
Does this test work with native .NET on Wine? I'd try myself, but winetricks dotnet4* doesn't seem to be working correctly anymore.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #3 from Esme Povirk madewokherd@gmail.com --- .NET Framework would have a different implementation anyway. A more pertinent question might be whether it works in Mono on Windows.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #4 from Zeb Figura z.figura12@gmail.com --- (In reply to Esme Povirk from comment #3)
.NET Framework would have a different implementation anyway. A more pertinent question might be whether it works in Mono on Windows.
Oh, are these tests of internal Mono classes?
What's the canonical platform for these tests? I'm failing to see how they can work on any platform, unless of course you have Wine bugs that effectively suppress TCP reset.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #5 from Esme Povirk madewokherd@gmail.com --- No, the tests are of public API's that pass on Windows with .NET Framework.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #6 from Esme Povirk madewokherd@gmail.com --- In almost all cases I consider .NET Framework on Windows to be the canonical platform. Any tests known to fail there are (with the exception of an error I discovered recently with the pinvoke3 tests) not run as part of the test suite on Wine. The intentional exception is 64-bit tests of XNA, since there is no 64-bit XNA on Windows.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #7 from Esme Povirk madewokherd@gmail.com --- What running tests in Mono on Windows would tell you is, assuming Mono and Wine Mono function similar (which for these API's I'd expect them to), whether the tests fail because of a difference between Mono and .NET Framework (in which case mscoree should be the component), or a difference between Wine and Windows.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #8 from Zeb Figura z.figura12@gmail.com --- (In reply to Esme Povirk from comment #7)
What running tests in Mono on Windows would tell you is, assuming Mono and Wine Mono function similar (which for these API's I'd expect them to), whether the tests fail because of a difference between Mono and .NET Framework (in which case mscoree should be the component), or a difference between Wine and Windows.
Sure, but why wouldn't Wine + .NET Framework tell you the same thing?
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #9 from Esme Povirk madewokherd@gmail.com --- It wouldn't tell you whether the thing Mono is doing would work on Windows, because .NET Framework might be doing something completely different.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #10 from Zeb Figura z.figura12@gmail.com --- (In reply to Esme Povirk from comment #9)
It wouldn't tell you whether the thing Mono is doing would work on Windows, because .NET Framework might be doing something completely different.
Err, to clarify, the "bug" I noticed in SocketResponder.cs is part of the tests, not part of the Mono implementation.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #11 from Esme Povirk madewokherd@gmail.com --- I'll look in more detail when I'm at work.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #12 from Esme Povirk madewokherd@gmail.com --- Double-checked and yes it passes in .NET Framework on Windows. It also passes in Mono on Windows, and it fails in .NET Framework on Wine.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #13 from Esme Povirk madewokherd@gmail.com --- I don't think the tests using SocketResponder care about what data the SocketResponder gets, except when requestHandler consumes the data, but it does seem like the expectation is that data sent by SocketResponder can be consumed on the other end, even though it does a Shutdown and Close without verifying that the data has been read.
Given that I haven't seen this specific failure mode before, I think it's unlikely there's a race condition on Windows, but maybe that could be tested by inserting a sleep() in the right place in one of the tests.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #14 from Zeb Figura z.figura12@gmail.com --- (In reply to Esme Povirk from comment #13)
I don't think the tests using SocketResponder care about what data the SocketResponder gets, except when requestHandler consumes the data, but it does seem like the expectation is that data sent by SocketResponder can be consumed on the other end, even though it does a Shutdown and Close without verifying that the data has been read.
Given that I haven't seen this specific failure mode before, I think it's unlikely there's a race condition on Windows, but maybe that could be tested by inserting a sleep() in the right place in one of the tests.
The problem is the other way around—the *client* (i.e. the server) is sending data that the *responder* never reads. I don't think a race would change anything—IIRC you get TCP reset regardless of whether the OS has received and buffered the data or not.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #15 from Zeb Figura z.figura12@gmail.com --- (In reply to Zeb Figura from comment #14)
the *client* (i.e. the server)
Please ignore that "i.e. the server" parenthesis; that was an editing error.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #16 from Esme Povirk madewokherd@gmail.com --- If the problem was that the server (responder) isn't reading the data, wouldn't that cause a client's write to fail?
The exceptions I see occurring in the few tests I've checked this for (using WINE_MONO_TRACE=E:System.Net.Sockets.SocketException) occur when the client attempts to read the socket.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #17 from Zeb Figura z.figura12@gmail.com --- (In reply to Esme Povirk from comment #16)
If the problem was that the server (responder) isn't reading the data, wouldn't that cause a client's write to fail?
Writes on a reset connection are supposed to fail, but in this case the write always happens before the shutdown. The zero-byte recv actually makes sure of that.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #18 from Esme Povirk madewokherd@gmail.com --- So why does it matter whether the server (responder) reads the data or not? Why would the server reading the data cause the client's read to succeed?
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #19 from Zeb Figura z.figura12@gmail.com --- (In reply to Esme Povirk from comment #18)
So why does it matter whether the server (responder) reads the data or not? Why would the server reading the data cause the client's read to succeed?
When one end of a TCP connection shuts down without consuming all its data, that's considered abnormal, and the OS sends an RST over the pipe. The other end notices that and reports a connection-reset via various APIs, including recv().
If the connection is shut down cleanly (i.e. all the data is read first), the shutdown end instead sends FIN; the client notices this and returns 0 from recv(), which is basically a special value that means the connection is closed.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #20 from Esme Povirk madewokherd@gmail.com --- Do we have tests for that behavior on Windows?
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #21 from Zeb Figura z.figura12@gmail.com --- (In reply to Esme Povirk from comment #20)
Do we have tests for that behavior on Windows?
test_tcp_reset() covers the mechanics of what happens when a TCP reset is received, but it uses a different mechanism for generating one. I don't think we have a test that closing a connection without reading everything in the pipe causes a TCP reset. But writing one up quickly confirms that's what happens.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #22 from Esme Povirk madewokherd@gmail.com --- I can modify the test code if you think that's the way to go. I just want to make sure this isn't going to pop up later in a real application where it'll be harder to debug.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #23 from Zeb Figura z.figura12@gmail.com --- Well, if it works with Mono on Windows, and fails with .NET on Wine, that's strong evidence that Wine is doing something wrong, so I think it deserves a second look on my part.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #24 from Zeb Figura z.figura12@gmail.com --- Created attachment 75644 --> https://bugs.winehq.org/attachment.cgi?id=75644 insanity
I really have no words.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #25 from Zeb Figura z.figura12@gmail.com --- [Ignore this comment if you don't want to read a lot of observation about sockets.]
On Windows, SD_RECEIVE sends RST immediately if there is data in the pipe, as well as if data is received after SD_RECEIVE. Closing the socket with data in the pipe also achieves this, or sending data to a closed connection.
Linux is slightly different. SHUT_RD never triggers RST, but closing a socket has about the same behaviour.
Reaction to RST is about the same across both operating systems, but there are subtle differences in the circumstances:
* RST on an open connection means that further I/O yields ECONNRESET. Linux is slightly different here in that recv() will actually consume any existing data in the pipe and *then* return ECONNRESET. I haven't tested how this interacts with other APIs like poll(). Windows effectively discards whatever's in the pipe.
* RST after FIN yields no error from recv()!
* RST after FIN but as a *response* to PSH (i.e. if a PSH is not ACKed in a packet which is itself not RST) yields no error on Linux. On Windows the send() succeeds (sure, it's asynchronous) but the following recv() yields ECONNABORTED. This case happens when we try to send data to a closed pipe (Windows and Linux are consistent about not acking the PSH here.)
On Linux, in any case when RST is received poll() returns POLLIN | POLLHUP | POLLERR. Note that the POLLIN occurs even if there is no data OR eof to read, i.e. even if the next recv() returns -1 / ECONNRESET. Also this poll() returns immediately even if there is data left in the pipe on Linux.
Windows has a million ways to poll for socket events, and I haven't tested them all. I will note at least that RST after FIN yields no error from WSAPoll() though.
When testing this initially, I was trying to emulate Mono's call sequence but inadvertently switched the order of the shutdown() calls. Astoundingly, this makes the difference. The above explains why: if we shutdown the write end first, that sends FIN before RST, and no error ever happens, but if we shutdown the read end first, that sends RST alone. On Wine of course any RST at all results in an error, because we use poll() [plus SO_ERROR] to detect it, and poll() always detects RST on Linux.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #26 from Zeb Figura z.figura12@gmail.com --- What is the upshot of all this? If we want to avoid reporting an error for RST after FIN, we have to specifically wait for FIN *at all times*. If we just poll as now, we don't have a way of knowing whether an RST happened after a FIN or not. [Note that Linux will return 0 on a reset socket once the error has been consumed.] This is not necessarily a problem, though.
The other upshot of all this is that, in trying to consume all the remaining data but using a 0-byte recv to do it, this test is definitely doing something broken. It works in practice, mind, but now that I've analyzed why it works, I think I can say with certainty that it works accidentally. If nothing else it's fragile, and should be fixed.
With that in mind, is it worth trying to fix this on the Wine end anyway? Maybe, but I'm not convinced. I can easily believe that a real application would do something this buggy, but on the other hand, we haven't seen one yet. And touching socket code has an astounding propensity to cause regressions, even if a great deal of them are simply because they expose other bugs. If nothing else I feel nervous about making a nontrivial change like this during code freeze.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #27 from Esme Povirk madewokherd@gmail.com --- Yeah I think it's fine to just change the tests. They're not designed to test for this weird quirk anyway.
https://bugs.winehq.org/show_bug.cgi?id=55952
--- Comment #28 from Esme Povirk madewokherd@gmail.com --- Fixed it in the Mono tests: https://github.com/madewokherd/wine-mono/commit/c55f9a74a46a74ae0dd8095e2f28...
Not sure what the resolution on this bug should be. FIXED because it was fixed in the tests (if so, wait for Wine Mono release?)? WONTFIX because Windows' behavior is impossible to reliably duplicate?
https://bugs.winehq.org/show_bug.cgi?id=55952
Zeb Figura z.figura12@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution|--- |ABANDONED
--- Comment #29 from Zeb Figura z.figura12@gmail.com --- (In reply to Esme Povirk from comment #28)
Not sure what the resolution on this bug should be. FIXED because it was fixed in the tests (if so, wait for Wine Mono release?)? WONTFIX because Windows' behavior is impossible to reliably duplicate?
I'd say ABANDONED because there is a Wine bug here, but the program was changed to not reproduce or need it anymore.
I think Windows' behaviour is possible to reliably duplicate here. I'm just not sure it's worth the effort, and the risk of regressions.
https://bugs.winehq.org/show_bug.cgi?id=55952
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #30 from Austin English austinenglish@gmail.com --- Closing.
-
WineHQ Bugzilla