Close Multi timers atexit. Add 1.6 CI

[IanButterworth]

Fixes #223

[vtjnash]

There should only be one function registered, since you cannot deregister them, this PR otherwise would create a memory leak

[codecov]

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (8a614d5) 90.62% compared to head (ab79e25) 89.70%.

Files	Patch %	Lines
src/Curl/Multi.jl	14.28%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #234      +/-   ##
==========================================
- Coverage   90.62%   89.70%   -0.92%     
==========================================
  Files           5        5              
  Lines         576      583       +7     
==========================================
+ Hits          522      523       +1     
- Misses         54       60       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[vtjnash]

Isn't there already a global list of these elsewhere

[StefanKarpinski]

Not that I'm aware of, no.

[vtjnash]

How does it reuse connections then if there is no list of the
? If you just close the Timer, it seems it might hang for ~forever instead of having the timer close the connection after 30 seconds?

[IanButterworth]

Should atexit instead call done!(multi)? Is there a global list of multi's already?

[IanButterworth]

The Multi constructor already calls done! on multi at the finalizer.

Downloads.jl/src/Curl/Multi.jl

Line 10 in 82b8da5

finalizer(done!, multi)

I guess the finalizer isn't being called before the trailing task warning?

[StefanKarpinski]

How does it reuse connections then if there is no list of the

Connections are only reused within a Downloader object; there's a single global default Downloader object. But if someone creates a new one of those locally, we don't have any global reference to it.

[vtjnash]

Ah, it sounds like there may need to be a global weakref list of active Downloader objects then?

[IanButterworth]

I see no warning with this locally now.

[StefanKarpinski]

Ah, it sounds like there may need to be a global weakref list of active Downloader objects then?

Yes, I think that Ian was trying to do this with a global (non-weak) list of timers to close. You think that doing a global weakref list of downloaders is a better approach? That always feels like such a failure of the programming language...

[IanButterworth]

@vtjnash I just noticed that on 1.10.0 this doesn't avoid the IO warnings, but it does on master. Has when atexit and the IO check changed on 1.10 vs. master?

[IanButterworth]

Ah JuliaLang/julia#51849 so this isn't a fix for 1.10

Indeed, CI shows the warning on 1.10

[IanButterworth]

@StefanKarpinski I guess it is actually worth dropping 1.3 because of these windows CI issues?

[StefanKarpinski]

Sure, go ahead and drop it. Not worth the effort anymore.

[IanButterworth]

Should be ready now