Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

channels test hanging issue #31507

Closed
vtjnash opened this issue Mar 27, 2019 · 4 comments · Fixed by #31673
Closed

channels test hanging issue #31507

vtjnash opened this issue Mar 27, 2019 · 4 comments · Fixed by #31673
Labels
regression Regression in behavior compared to a previous version

Comments

@vtjnash
Copy link
Member

vtjnash commented Mar 27, 2019 •
edited
Loading

see e.g https://ci.appveyor.com/project/JuliaLang/julia/builds/23345251/job/9ts99c8wym4toiht

been seeing this for a couple days at least

EDIT: I think this seems likely to be related to #30186 in some way
@vtjnash vtjnash added system:windows Affects only Windows regression Regression in behavior compared to a previous version labels Mar 27, 2019
@vtjnash
Copy link
Member Author

vtjnash commented Mar 27, 2019 •
edited
Loading

fwiw, I believe the culprit is #30806

@vtjnash
Copy link
Member Author

vtjnash commented Mar 28, 2019

probably something older, as it also seems to have happened on older builds https://ci.appveyor.com/project/JuliaLang/julia/builds/21512596/job/ym5ol1k2kjg0lg29
https://ci.appveyor.com/project/JuliaLang/julia/builds/21037773/job/opaj4joh6v7406jl
https://ci.appveyor.com/project/JuliaLang/julia/builds/20832326/job/025iqjrfdlueum18

possibly further back than
https://ci.appveyor.com/project/JuliaLang/julia/builds/20355614/job/7sjrnm9asej5to38
even (or that's a different issue, since it's something else that's hung that time)

@vtjnash
Copy link
Member Author

vtjnash commented Mar 29, 2019

OK, maybe multiple issues here. I downloaded the AV logs, and it's been hanging for a long time:
https://ci.appveyor.com/project/JuliaLang/julia/builds/7261861/job/kob4b2axe33p6uv5 (remember those days, when we could test the whole thing faster than we can run just the Pkg tests today?)

But also most of those I've spot checked passed the channels test, so that might be new.

@vtjnash
Copy link
Member Author

vtjnash commented Apr 3, 2019

Probably also happened on FreeBSD: https://build.julialang.org/#/builders/29/builds/895

@vtjnash vtjnash changed the title channels test hangs on windows channels test hanging issue Apr 3, 2019
@vtjnash vtjnash added this to the 1.2 milestone Apr 3, 2019
@vtjnash vtjnash removed the system:windows Affects only Windows label Apr 3, 2019
vtjnash added a commit that referenced this issue Apr 10, 2019
@vtjnash
channels: remove WeakRef from Condition
66cb288 
Using a WeakRef meant we might not actually `bind` the result.
If nobody was still holding a reference to put contents into the Condition,
we would simply garbage collect it, and then never need to close it.
Since that does not seem to be the intent,
instead move to just keeping a strong reference
(alternatively, we would have to switch to using `stream_wait`
with ref-counting, but that seems suboptimal for several reasons.).

fix #31507
@vtjnash vtjnash mentioned this issue Apr 10, 2019
Merged
vtjnash added a commit that referenced this issue Apr 11, 2019
@vtjnash
channels: remove WeakRef from Condition
359258b 
Using a WeakRef meant we might not actually `bind` the result.
If nobody was still holding a reference to put contents into the Condition,
we would simply garbage collect it, and then never need to close it.
Since that does not seem to be the intent,
instead move to just keeping a strong reference
(alternatively, we would have to switch to using `stream_wait`
with ref-counting, but that seems suboptimal for several reasons.).

fix #31507
JeffBezanson pushed a commit that referenced this issue Apr 11, 2019
@vtjnash @JeffBezanson
channels: remove WeakRef from Condition (#31673)
29f61cd 
Using a WeakRef meant we might not actually `bind` the result.
If nobody was still holding a reference to put contents into the Condition,
we would simply garbage collect it, and then never need to close it.
Since that does not seem to be the intent,
instead move to just keeping a strong reference
(alternatively, we would have to switch to using `stream_wait`
with ref-counting, but that seems suboptimal for several reasons.).

fix #31507
KristofferC pushed a commit that referenced this issue Apr 15, 2019
@vtjnash @KristofferC
channels: remove WeakRef from Condition (#31673)
95e2a54 
Using a WeakRef meant we might not actually `bind` the result.
If nobody was still holding a reference to put contents into the Condition,
we would simply garbage collect it, and then never need to close it.
Since that does not seem to be the intent,
instead move to just keeping a strong reference
(alternatively, we would have to switch to using `stream_wait`
with ref-counting, but that seems suboptimal for several reasons.).

fix #31507

(cherry picked from commit 29f61cd)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
regression Regression in behavior compared to a previous version
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

channels: remove WeakRef from Condition JuliaLang/julia
1 participant
@vtjnash