Requests to AWS S3 hang with HTTP.jl v0.9.15

[nickrobinson251]

With [email protected], i'm seeing GET requests to S3 hang indefinitely.

The actual behaviour i am seeing is using AWSS3.jl v0.9.2 (example at JuliaCloud/AWSS3.jl#215), but the behaviour is only present when using HTTP v0.9.15.

I haven't quite narrowed down what's going on... but here's a bit of thinking out loud:

• AWSS3.s3_get_file calls get_object (which is a AWS.jl call)
• which in turn calls AWS.s3("GET", "/$(Bucket)/$(Key)"; aws_config)
• which is calling a AWS.RestXMLService object...
• which finally constructs a HTTP.Request and calls HTTP.request
  • construct HTTP.Request: https://github.com/JuliaCloud/AWS.jl/blob/564f9c55291a29c6994bf39625912256bf2e9c4f/src/AWS.jl#L223-L232)
  • via submit_request: https://github.com/JuliaCloud/AWS.jl/blob/9a37112f63cc38a59f6b255bb1aff74fc697eec5/src/utilities/request.jl#L59-L84
  • calls HTTP.request: https://github.com/JuliaCloud/AWS.jl/blob/9a37112f63cc38a59f6b255bb1aff74fc697eec5/src/utilities/request.jl#L186-L196

Looking at the 0.9.15 changes (v0.9.14...v0.9.15) i'd guess the most related change might be #752 ?

Needs further investigation :)

[fredrikekre]

Most likely yea, does it work again if you close the response stream as suggested in the CHANGELOG? Might have been a too breaking change to put in a feature release.

[fredrikekre]

Fortunately you can call close multiple times so if you add that it should be compatible with old HTTP.jl still.

[pfitzseb]

#517 is very similar.

[nickrobinson251]

does it work again if you close the response stream

i'll give it a go :)

it didn't fix it on my first attempt, but i'm not too familiar with the AWS code so might need to add a close call in multiple places -- taking a look now.

[oxinabox]

I would guess what is happening is that somewhere in the code for AWS / AWSS3
something was calling some other function that will keep reading until it encounters a EOF, and will block if it gets to the end of the steam without hitting the end of file. (Which is a feature of some read commands, that might be used on IOSteams where things trickle in slowly).
And since HTTP.jl was, after #752, no longer closing the stream, it was now hanging waiting for a closing of the steam that would never come.

This makes me think that #752 was not the right solution to #543
If we know the data is done arriving, we should be closing the stream in HTTP.
But i guess not having Base.closewrite means it is impossible to indicate that writing is done without also preventing reading.

It might still be better as a hack to close all streams except IOBuffer or something like that?
Until we have closewrite?
cc @c42f

[omus]

It should be pointed out that using response_stream for HTTP.jl 0.9.14 and below only really worked (I believe) with the unexported Base.BufferStream. The implementation of this stream seems to require the it be be closed in order for it to be read from. Most streams don't work this way.

I think the general rule should be that you shouldn't close IO instances that you didn't create.

[fredrikekre]

But read would still block, e.g. this code now hangs:

using HTTP, SimpleBufferStream, SHA
bs = BufferStream()
HTTP.get("https://julialang.org"; response_stream=bs)
SHA.sha256(bs)

I think the general rule should be that you shouldn't close IO instances that you didn't create.

Yea, this makes sense, but perhaps it was a bit too optimistic to put this in a feature release.

[fredrikekre]

I propose we revert the #752 change: #773. In addition to breaking AWS it also breaks tests for SimpleBufferStreams and TeeStreams for example.

[omus]

I propose we revert the #752 change: #773. In addition to breaking AWS it also breaks tests for SimpleBufferStreams and TeeStreams for example.

We should make a follow up issue to remove the close call in a breaking release (1.0.0)