d2 icon indicating copy to clipboard operation
d2 copied to clipboard

watch breaks if chrome is started for d2 and still listening

Open Vaelatern opened this issue 3 years ago • 5 comments

$ cat test.d2
a -> b -> c
$ d2 --layout=elk -w test.d2

It compiles just fine.

I opened test.d2 in an editor and made a change.

I changed a -> b -> c to a -> b -> c -> d

On stdout I see [13:06:54] info: detected change in test.d2: recompiling... but it never completes. I can refresh and check the file on disk, the graph doesn't include d. If I restart, then it compiles just fine and I see the changes then.

I've confirmed this on no-op changes too.

EDIT: When I close the browser that got opened, the recompile finishes just fine. If I close the initial browser and point a new browser at the link, the recompile loop works exactly as it's supposed to.

This is the first time I've let d2 launch the browser itself. Perhaps this was always how it worked?

Vaelatern avatar Dec 19 '22 18:12 Vaelatern

@Vaelatern hmm i just tested and it works here, with or without a browser window. is this consistently reproduce-able? what's your OS? does d2 version show 0.1.2?

alixander avatar Dec 19 '22 18:12 alixander

My OS is Void, the browser being opened is chromium, d2 version is v0.1.2-HEAD

It is consistently reproducable, I saw it on a large diagram I drew and found a minimal case to prove it

I've confirmed it happens on 0.1.1 too. Changing topic of issue.

Vaelatern avatar Dec 19 '22 18:12 Vaelatern

I think it's possible that it breaks if the command to trigger the browser is not short. I normally don't use chrome, but it's started just for the build. As soon as chrome is quit, the build proceeds.

Vaelatern avatar Dec 19 '22 18:12 Vaelatern

can you try with BROWSER=0?

alixander avatar Dec 19 '22 18:12 alixander

Works just fine. It seems to only hang when it is the originator of the browser session.

Vaelatern avatar Dec 19 '22 18:12 Vaelatern

The core problem is this can be a blocking command: https://github.com/terrastruct/util-go/blob/master/xbrowser/xbrowser.go#L27

Vaelatern avatar Dec 25 '22 05:12 Vaelatern

I suppose we could start BROWSER and disown it instead of waiting.

You can also try adding a wrapper script like so:

async-chromium.sh

#!/bin/sh
set -eu
chromium "$@" &
# You might need this to ensure chromium continues to run after this script exits.
# disown

And pointing BROWSER to it. Like BROWSER=/path/to/async-chromium.sh.

nhooyr avatar Dec 25 '22 06:12 nhooyr

To clarify, are you setting BROWSER to chromium or do you have it empty?

If you have it empty, then we'll need to fork the third party browser package we import. Only $BROWSER we implement.

nhooyr avatar Dec 25 '22 06:12 nhooyr

BROWSER is empty. It must be doing an xdg-open $URL for this behaviour (I never use chromium, and it's still my system default, so my setup was perfect for stumbling across this).

Vaelatern avatar Dec 26 '22 01:12 Vaelatern

Shouldn't xdg-open be async?

nhooyr avatar Dec 26 '22 16:12 nhooyr

Here's the executables it tries on linux: https://github.com/pkg/browser/blob/681adbf594b8314eb76d83a271f4a1c152d057d9/browser_linux.go#L9

nhooyr avatar Dec 26 '22 16:12 nhooyr

Most people have a browser open already.

When a browser is open already, there is an expected behaviour for firefox and chromium and probably downstream Google Chrome too. This is: send the URL to be opened to the existing browser context and exit. This is a short lived invocation, usually under a second. This makes sense: browsers share state directories and databases heavily, so you want to send URLs to be opened by the existing browser instance if it exists.

When there is no existing browser instance, those two browsers I mentioned and tested will remain open, not forking to background. This makes sense: there is little sense in forking to background just because you will be long lived.

I was able to trigger this bug because the xdg handler for urls on my system is chromium. Meanwhile, I only use chromium for one-off usage, daily driving firefox. Therefore there is no existing browser context when d2 tries to open a window. Thus chromium is started, and since there isn't an existing chromium context, d2 is blocked waiting for the executable it started to finish.

There is no provision nor rationale for xdg-open to fork everything it starts to background. It would break the standard handling of child processes.

Vaelatern avatar Dec 26 '22 21:12 Vaelatern

There is no provision nor rationale for xdg-open to fork everything it starts to background. It would break the standard handling of child processes.

I'm surprised. I'd say the rationale is consistent usage of xdg-open in scripts and whatnot. And across OSes, macOS's open for example always just opens the app and exits. Are you sure this isn't specific to your setup in some way?

I can't see how other people would effectively use github.com/pkg/browser on Linux if it blocked opening a URL when a browser wasn't already open.

I'm also unable to find any docs on whether xdg-open will block starting a new application. Though I'm unable to find any explicit mention of it opening in the background either.

It would break the standard handling of child processes.

I don't know what you mean by this. What do you consider the standing handling of child processes?

nhooyr avatar Dec 27 '22 04:12 nhooyr

I don't know what you mean by this. What do you consider the standing handling of child processes?

Start in the foreground, don't daemonize unless told to do so, and clean up your children before you exit.

Are you sure this isn't specific to your setup in some way?

This has been the behaviour on my systems as long as I can remember. As for being unique to my own setup, would you consider an ubuntu reproduction in a clean VM sufficient proof? I can prepare one if you like.

Vaelatern avatar Dec 27 '22 06:12 Vaelatern

Yes please do give it a shot in Ubuntu!

nhooyr avatar Dec 27 '22 09:12 nhooyr

Ok, in a bog standard ubuntu install, it works as you surmised, xdg-open itself forking the browser to background.

Turns out, on reading xdg-open, there is a thing it does in opening: it tries to pawn off the opened program to the desktop. I'm only running a window manager, hence why this behaviour is consistent among all my machines, but not on a fresh ubuntu install (with their Gnome running).

Vaelatern avatar Dec 27 '22 17:12 Vaelatern

Ah got it. I don't know what the correct solution would be here. But you can try the following script in /usr/local/bin/xdg-open:

#!/bin/sh
nohup /usr/bin/xdg-open "$@" >/dev/null 2>&1 &

It'll be unfortunate that you won't be able to know if xdg-open succeeded or failed but it'll at least prevent the blocking. And you can always run /usr/bin/xdg-open manually to debug.

nhooyr avatar Dec 27 '22 18:12 nhooyr