rdeits / MeshCat.jl

WebGL-based 3D visualizer in Julia
MIT License
234 stars 43 forks source link

with_logger(NullLogger()) breaks MeshCat #68

Closed tkoolen closed 4 years ago

tkoolen commented 6 years ago

I wanted to make MeshCat/the web stack a little less verbose, so I tried using with_logger(NullLogger()):

julia> using MeshCat

julia> vis = Visualizer()
MeshCat Visualizer with path /meshcat

julia> using Logging

julia> with_logger(NullLogger()) do
           open(vis)
       end
Process(`xdg-open http://127.0.0.1:8700`, ProcessExited(0))

Unfortunately, only the first time I run open(vis), I get

image

If I reload the browser tab, it's fine. Should open include a wait or something?

rdeits commented 6 years ago

Uh, fascinating. Yeah, I think this is just a symptom of some broader issues with deciding when to be synchronous vs. asynchronous. I'll take a look at this issue specifically, though.

tkoolen commented 5 years ago

Alright, so

https://github.com/rdeits/MeshCat.jl/blob/d93d0a7d5ad262f572ec0cca42d9c96b1c134fed/src/servers.jl#L5

calls WebIO.webio_serve, which calls Mux.serve, which is @async anyway, so I think the @async in open isn't necessary. But the question still remains how to assure that the web server is up and running before calling open_url. If we could just create the HTTP.Server ourselves, we could probably do that using isopen(::Server), right?

rdeits commented 5 years ago

I think we might still need the @async, because webio_serve actually calls this Mux.serve method: https://github.com/JuliaWeb/Mux.jl/blob/849d72c245f0ba32c8f856b27a393d5aa7b99e5b/src/server.jl#L65 which in turn calls https://github.com/JuliaWeb/WebSockets.jl/blob/0aaca05824df7a02c5feaca910453a63083877c0/src/HTTP.jl#L359 whish is not async.

I wonder if we could copy that WebSockets.serve() method into MeshCat, but make the HTTP.listen call async and return the tcpserver.

rdeits commented 5 years ago

This seems to work, and might be a suitable replacement for webio_serve (while also returning the underlying TCPServer. It's basically just a combination of the relevant webio_serve, Mux.serve and WebSockets.serve methods, with a bit of cleanup:

using WebIO
using WebSockets
using Mux
using HTTP
using Sockets

app = Mux.page("/", req -> "hello")

http = Mux.App(Mux.mux(Mux.defaults, app, Mux.notfound()))
websock = Mux.App(Mux.mux(
    Mux.wdefaults,
    Mux.route("/webio-socket", WebIO.create_socket),
    Mux.wclose,
    Mux.notfound(),
))

server = WebSockets.ServerWS(Mux.http_handler(http), Mux.ws_handler(websock))
host = Mux.localhost

function serve(server::WebSockets.ServerWS{T, H, W}, host, port, verbose) where {T, H, W}

    tcpserver = Ref(Sockets.listen(Sockets.InetAddr(host, port)))

    @async begin
        take!(server.in)
        close(tcpserver[])
    end

    function _servercoroutine(stream::HTTP.Stream)
        try
            if WebSockets.is_upgrade(stream.message)
                WebSockets.upgrade(server.wshandler.func, stream)
            else
                WebSockets.handle_request(server.handler.func, stream)
            end
        catch err
            put!(server.out, err)
            put!(server.out, stacktrace(catch_backtrace()))
        end
    end

    @async HTTP.listen(_servercoroutine,
            host, port;
            tcpref=tcpserver,
            ssl=(T == HTTP.Servers.https),
            sslconfig = server.options.sslconfig,
            verbose = verbose,
            tcpisvalid = server.options.ratelimit > 0 ? checkratelimit! :
                                                     (tcp; kw...) -> true,
            ratelimits = Dict{IPAddr, HTTP.Servers.RateLimit}(),
            ratelimit = server.options.ratelimit)
    return tcpserver[]
end

port = 8004
tcpserver = serve(server, host, port, false)
run(`xdg-open http://localhost:$port`)
tkoolen commented 5 years ago

Ah right. I was assuming that the WebIO.serve methods all funnel into one.

I'm a little bit worried about this being brittle; it's quite a bit of additional code that we need to keep in sync with the WebIO stack. But maybe that's just what's needed. Another option could be to open a few PRs against various web stack packages (man there's a lot of them).

Does this really work by the way? Since the tcpserver Ref is assigned to in the @async listen call, isn't there a chance that it's still Ref(nothing) when you run xdg-open?

I was wondering if we could just use the first argument to webio_serve to somehow set up a Channel that we can listen to, but I think the relevant callable is only called upon a request.

rdeits commented 5 years ago

Yeah, I'd prefer to get this into WebSockets instead of having to maintain it ourselves, but we can at least try it out and see if it works.

I think this should work, although we might need to wait until isopen(tcpserver) == true. There's no danger of it being nothing in the implementation I pasted above: I'm constructing and assigning the ref all at once, while the implementation in WebSockets.jl did leave it unassigned until the call to listen. As far as I can tell, the only reason that's done is to allow reusing an existing server or port, which isn't something we need to do here.

tkoolen commented 5 years ago

I'm constructing and assigning the ref all at once.

Oh yeah, I hadn't noticed you'd changed that.