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.

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

Change implementation of wait. #99

Merged
merged 1 commit into from
Jul 8, 2019
Merged

Change implementation of wait. #99

merged 1 commit into from
Jul 8, 2019

Conversation

twavv
Copy link
Contributor

@twavv twavv commented Apr 6, 2019

The current implementation is broken because connections are lazily added to ConnectionPool.connections. Related to (but not dependent on) JuliaGizmos/WebIO.jl#265.

@codecov-io
Copy link

codecov-io commented Apr 6, 2019

Codecov Report

Merging #99 into master will increase coverage by 0.22%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #99      +/-   ##
==========================================
+ Coverage    73.2%   73.43%   +0.22%     
==========================================
  Files          12       12              
  Lines         321      320       -1     
==========================================
  Hits          235      235              
+ Misses         86       85       -1
Impacted Files Coverage Δ
src/visualizer.jl 75.71% <0%> (+1.06%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f6771c...a8b9d31. Read the comment docs.

@tkoolen
Copy link
Collaborator

tkoolen commented Apr 9, 2019

I tried this locally, and it seems to block forever. The code I ran (pasted into the REPL):

using MeshCat
begin
    vis = Visualizer()
    open(vis)
    println("here")
    wait(vis)
end

Note that it does work if you paste this line by line without the begin block.

Edit: this is with WebIO 0.7.

@twavv
Copy link
Contributor Author

twavv commented Apr 10, 2019

Strange. I don't think the implementation of how connections are added changed between 0.5/0.7 and now.

Possibly merge this and bump WebIO requirement when next WebIO is released?

@tkoolen
Copy link
Collaborator

tkoolen commented Apr 11, 2019

I can confirm that this doesn't block forever with WebIO master. But since there shouldn't have been any changes, I'm hoping this isn't just 'fixed' because of a race condition that happens to get triggered on 0.7 and not on master.

@rdeits rdeits mentioned this pull request Jul 1, 2019
@rdeits rdeits merged commit a8b9d31 into rdeits:master Jul 8, 2019
@rdeits
Copy link
Owner

rdeits commented Jul 8, 2019

I included this with the update to WebIO 0.8 in #109 but it seems to still block forever under some circumstances.

For example, this works:

julia> using MeshCat, Test

julia> @testset "foo" begin
           vis = Visualizer()
           open(vis)
           wait(vis)
       end

but this blocks forever:

julia> @testset "foo" begin
           vis = Visualizer()
           setobject!(vis, Triad(1.0))
           open(vis)
           wait(vis)
       end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants