1 Aug
2016
1 Aug
'16
2:41 p.m.
Hi Hans,
On 21.07.2016 16:32, Hans Leidekker wrote:
Although it hangs on current Wine, it doesn't on Windows, and it shows
that blocking reads should not be used here.
Thanks,
Jacek
[1] http://source.winehq.org/patches/data/125006
On Thu, 2016-07-21 at 14:25 +0200, Jacek Caban wrote:
This is required to make get_avail_data return the correct
number of
bytes. Note that the line above sets the read size to 0, so (before my
second patch) the next call to get_avail_data would return 0. This may
trigger a read beyond available data. It is what happens with Outlook.
Again, refill_read_buffer usually takes care of that. I think it
probably didn't work in your case due to quirks we needed for
non-blocking reads.
I sent a patch that should improve chunked stream behaviour a lot [1].
There is still more work that could be done (like improvement of
chunked_get_avail_data or taking advantage of now properly working
non-blocking reads to improve InternetRead* functions), but I expect it
to fix the problem that you were trying to solve with this patch.
It's likely that your patch 3/3 may also not be needed (and if it is, it
will need adjustment for the new way of storing chunk size).
---
dlls/wininet/http.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c
index 2c62d97..c4cf66b 100644
--- a/dlls/wininet/http.c
+++ b/dlls/wininet/http.c
@@ -2921,6 +2921,7 @@ static DWORD set_content_length(http_request_t *request)
!strcmpiW(encoding, szChunked))
{
chunked_stream_t *chunked_stream;
+ DWORD res;
chunked_stream = heap_alloc(sizeof(*chunked_stream));
if(!chunked_stream)
@@ -2935,6 +2936,12 @@ static DWORD set_content_length(http_request_t *request)
memcpy(chunked_stream->buf, request->read_buf+request->read_pos,
request->read_size);
chunked_stream->buf_size = request->read_size;
request->read_size = request->read_pos = 0;
+
+ res = start_next_chunk(chunked_stream, request);
+ if (res != ERROR_SUCCESS) {
+ heap_free(chunked_stream);
+ return res;
+ }
It seems to me that you're fixing a problem is a wrong place.
start_next_chunk should be called by HTTP_ReceiveRequestData via
refill_read_buffer anyway. I'd suggest to investigate why it doesn't
work in your case. Also, see the
attached test. We shouldn't need any chunk to complete
requests. It's an existing problem in current Wine, which may be
unrelated to your problem, but having such blocking calls here is not a
step in the right direction.
Your test hangs with and without this patch. Why is
having a blocking
call here a problem?