[2/3] wininet: Start the first chunk as soon as we have some data.

[Jacek Caban]

Hi Hans,

On 21.07.2016 16:32, Hans Leidekker wrote:
> On Thu, 2016-07-21 at 14:25 +0200, Jacek Caban wrote:
>>> ---
>>>  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.
> 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).

>> 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?

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