Avoid potential race between read and upload_data

[gbaraldi]

upload_data might set easy.input to nothing before it's actually read if read_callbacks runs before it.

[codecov[bot]]

Codecov Report

Merging #231 (7531f74) into master (246504e) will decrease coverage by 0.34%. Report is 1 commits behind head on master. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #231      +/-   ##
==========================================
- Coverage   90.26%   89.93%   -0.34%     
==========================================
  Files           5        5              
  Lines         575      576       +1     
==========================================
- Hits          519      518       -1     
- Misses         56       58       +2     

Files Changed	Coverage Δ
src/Curl/Easy.jl	91.58% <100.00%> (-0.65%)	:arrow_down:

... and 3 files with indirect coverage changes

[vtjnash]

Do you have an example? That sounds like a much more severe data race than this change would fix, but I don't see how you could encounter that situation, since read_callbacks should only occur after this method calls CURLPAUSE_CONT.

[gbaraldi]

So I've hit this in the IO thread PR by just doing

using Downloads

url = "https://httpbingo.julialang.org/put"
file = tempname()
write(file, "Hello, world!")
resp = nothing
body = sprint() do output
    resp = request(url; output=output, input=file)
end

I'm still grasping my way around this codebase, but I don't see a way that read_callbacks is forced to run after upload_data has run. It might also be a bug on the IO thread PR, but the other issues i've seen so far are those that use the running of the IO Loop as an implicit synchronization step.

[StefanKarpinski]

@gbaraldi, what do you think of @vtjnash's latest version of this? Does it fix the issue for you? I'm happy to merge (and backport) whatever version this you guys settle on.

[vchuravy]

We discussed this during the multi-threading WG call. I just tested locally and it looks good. Deployed this to https://github.com/JuliaLang/julia/pull/50880 and we will see what Base CI says.

[vchuravy]

@StefanKarpinski this looks good on Base CI. This does require Julia 1.8 though, how do you want to handle that?

[StefanKarpinski]

I guess we don't backport to 1.6 then. This becomes a fix only for newer versions.

[vchuravy]

So should I just remove the CI for 1.3? Or do you want a VERSION >= v"1.8.0" check?