leakec
commented
9 months ago @imsnif Any ideas on what may be causing this?
Closed leakec closed 6 months ago
leakec
commented
9 months ago @imsnif Any ideas on what may be causing this?
imsnif
commented
9 months ago Does it happen if you give it an absolute path without the $HOME?
leakec
commented
9 months ago @imsnif Yeah, I'm still seeing it even with an absolute path rather than $HOME.
imsnif
commented
9 months ago Hey yeah - I looked into this a little bit. I think some event ordering changed in the recent version and this plugin was relying on this ordering (my bad!) - a good way to fix this might be to make opening the multitask environment atomic. So that if we did it once, we will never do it again until the plugin is reloaded. Maybe some conditional around here: https://github.com/imsnif/multitask/blob/main/src/main.rs#L64
leakec
commented
9 months ago @imsnif I gave this a shot and also added some print statements to get some more info. I'm using a bool on the state to try and ensure we only open the plugin once. Here is the diff:
diff --git a/src/main.rs b/src/main.rs
index ed353ac..09009bd 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -25,6 +25,7 @@ struct State {
plugin_id: Option<u32>,
shell: String,
cwd: Option<PathBuf>,
+ was_opened: bool,
}
impl ZellijPlugin for State {
@@ -45,6 +46,8 @@ impl ZellijPlugin for State {
Some(s) => Some(PathBuf::from(s)),
_ => None
};
+ eprintln!{"RUNNING LOAD for ID {}", self.plugin_id.unwrap()};
+ eprintln!{"Zellij PID: {}",get_plugin_ids().zellij_pid}
show_self(true);
}
@@ -61,7 +64,14 @@ impl ZellijPlugin for State {
// we clean up our state and start over, allowing the plugin to be triggered by
// a keybinding
hide_self();
- self.start_multitask_env();
+ eprintln!{"RUNNING PANE UPDATE for ID {}", self.plugin_id.unwrap()};
+ eprintln!{"Status of was opened: {}", self.was_opened};
+ if !self.was_opened
+ {
+ self.was_opened = true;
+ eprintln!{"Status of was opened: {}", self.was_opened};
+ self.start_multitask_env();
+ }
} else if let Some(running_tasks) = &mut self.running_tasks {
running_tasks.update_task_status(&pane_manifest);
if running_tasks.all_tasks_completed_successfully() {However, I'm still seeing the same issue.
I checked the log to see the status of my print statements:
DEBUG |/home/leake/.zellij_plugi| 2023-11-11 09:02:21.773 [id: 2 ] RUNNING LOAD for ID 2
DEBUG |/home/leake/.zellij_plugi| 2023-11-11 09:02:21.773 [id: 2 ] Zellij PID: 37983
ERROR |??? | 2023-11-11 09:02:21.773 [screen ] [/home/leake/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zellij-server-0.39.0/src/screen.rs:822]: a non-fatal error occured
Caused by:
0: Failed to switch to active tab at position 1 for client id: 2
1: active tab not found for client 2
INFO |zellij_server | 2023-11-11 09:02:21.774 [async-std/runti] [/home/leake/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zellij-server-0.39.0/src/lib.rs:917]: Compiling plugins using Cranelift
DEBUG |/home/leake/.zellij_plugi| 2023-11-11 09:02:21.792 [id: 2 ] RUNNING LOAD for ID 2
DEBUG |/home/leake/.zellij_plugi| 2023-11-11 09:02:21.792 [id: 2 ] Zellij PID: 37983
DEBUG |/home/leake/.zellij_plugi| 2023-11-11 09:02:21.795 [id: 2 ] RUNNING PANE UPDATE for ID 2
DEBUG |/home/leake/.zellij_plugi| 2023-11-11 09:02:21.795 [id: 2 ] Status of was opened: false
DEBUG |/home/leake/.zellij_plugi| 2023-11-11 09:02:21.795 [id: 2 ] Status of was opened: true
DEBUG |/home/leake/.zellij_plugi| 2023-11-11 09:02:21.795 [id: 2 ] RUNNING PANE UPDATE for ID 2
DEBUG |/home/leake/.zellij_plugi| 2023-11-11 09:02:21.795 [id: 2 ] Status of was opened: false
DEBUG |/home/leake/.zellij_plugi| 2023-11-11 09:02:21.795 [id: 2 ] Status of was opened: true It looks like the call to load was made twice for plugin ID 2, which ultimately means two calls are made to the pane update. I can see within the pane update that the state of was_opened is changed correctly, but when the second pane update fires the was_opened variable is still set to false. I'm guessing there is either:
State works. This could definitely be an error on my part as I'm new to Rust and potentially have made some rookie mistake. Maybe using the plugin State wasn't what you had in mind and I misinterpreted your recommendation (sorry if that is the case).State.One other interesting thing to note. If I use start-or-reload-plugin rather than launch-or-focus-plugin the issue goes away. In the logs, I also only see one call to load and one call to the pane update. However, it seems like the --configuration is not applied correctly when I re-open the plugin, e.g., if I change the shell variable on subsequent calls to start-or-reaload-plugin it still has the same value it had from the first initial call.
leakec
commented
8 months ago @imsnif I compiled zellij from GitHub using the devel instructions so I could start to dump stack traces and try to figure out why the plugin was being loaded twice. I'm very unfamiliar with the internals of zellij, so I'm doing some speculating in the paragraphs that follow: please take it all with a huge grain of salt. I also have the stack traces from the zellij log at the end if you're interested.
The load_plugin command in zellij-server/src/plugins/wasm_bridge.rs is only being called once. However, when I added a stack trace on get_store in zellij-server/src/lib.rs I saw two separate stack traces, which means that it is getting called twice. I chose get_store since I saw that the plugin looked like it was being compiled twice (I think leading to it being loaded twice and ultimately two multitask windows being loaded).
It looks like create_plugin_environment is called once during start_plugin and then again by clone_instance_for_other_clients (which I think makes sense since each client would need to start the plugin). Is it possible that zellij has itself as a client so it is creating the plugin environment twice?
EDIT: Also including stack trace from start-or-reload-plugin, which only calls get_store once. As a result, load is only called once and we only get one editor window opening as expected. Maybe what is happening is launch-or-focus-plugin is calling with connected_clients while start-or-reload-plugin is not. Could there be an issue with which clients are passed by the LaunchOrFocusPlugin command?
After looking through these stack traces again, the one from start or reload calls load_plugin on 224 of zellij-server/src/plugins/mod.rs while lanch or focus calls it at line 187. The biggest difference I'm noticing here is cwd and ClientId are None in the former but are values in the later.
Did some more printing and looking at logs today. When reloading a plugin, Some(client_id) is passed in Load vs. None being passed in when calling Reload. Reload is the one that works as expected, i.e., None works as expected but Some(client_id) doesn't. The None value being passed in ultimately results in the client_id being the same as the first connected client, which is correct. The Load method is receiving a client_id method that is different than the connected client, which results in the plugin being loaded twice.
Trying to trace where this comes from: It looks like PluginInstruction::Load is only called on 1346 of zellij-server/src/pty.rs in fill_plugin_cwd; fill_plugin_cwd is only called on line 657 of the same file, which is part of the main pty loop. It looks like this command is only called by functions in zellij-server/src/screen.rs, for example ScreenInstruction::LaunchOrFocusPlugin. This is only called in route_action in zellij-server/src/route.rs. The route_thread_main is the only function calling the route_action, which is started by the zellij server. I'm not sure how this could lead to a client_id of 2, since it seems like the client_id should ultimately be inherited from the route_thread_main's client_id. Unless somehow new_client is being called and giving a new route_thread_main?
Put some prints in route_thread_main today and indeed it is being called again, with a client_id=2 when using launch-or-focus-plugin. It also has client_id=2 when calling start-or-reload-plugin. The difference is start-or-reload-plugin ultimately calls Reload, which passes client_id=None onto the plugin loader, whereas launch-or-focus-plugin ultimately calls Load, which passes the client_id=2 on, which results in the two loads. Why does this route_thread_main have an id of 2? Figured it out, whenever we call a command from within a zellij session using the zellij command, e.g., zellij edit <file>, this also happens. Basically, the zellij command we are running creates a new client.
Coming from the other side (CLI) I see CliAction::LaunchOrFocusPlugin in zellij-utils/src/input/actions.rs
Left off here, will continue later.
leakec
commented
8 months ago @imsnif Just sending a gentle reminder for the message above. I'm curious if this could be something in the zellij core that is causing the plugin to be loaded twice.
imsnif
commented
8 months ago Hey, I'm really sorry for taking so long to get to this. As always, I'm incredibly swamped and don't have enough resources to deal with everything that needs to be dealt with in Zellij.
I hope to get to this soon. If you'd like to do some debugging with Zellij itself and find out what's happening, that would be great. I realize this is a tall order though. So if you'd rather wait for me, I can only ask you for your patience and thank you for your consistency. I have not forgotten you, I'm however only one person :/
leakec
commented
8 months ago @imsnif No sweat at all. Just wanted to send a reminder, since I know it's easy for stuff to get buried. Thanks again for all the amazing work you do with zellij!
I'm a bit swamped at work as well (so have had less time for this than I'd like). I can try to do some debugging if I get some spare time over the next few weeks, but would appreciate any help you can provide whenever you have the chance (no rush).
leakec
commented
8 months ago @imsnif Okay, I figured out what is happening, but am not sure of the correct fix. Again, no rush on this, just wanted to put this message here as a nice summary whenever you get around to it (so you don't have to read through my rambling in the full comment).
zellij from within a session e.g., zellij edit <file> or zellij action <action>, it creates a new client. This comes in the form of a new listener, which spins up a new route_thread_main with a new client ID.start-or-reload-plugin, the client_id of route_thread_main is never passed on to the WASM bridge that loads the plugin. Hence, this plugin is only loaded for the connected_clients, and is only called once.launch-plugin or launch-or-focus-plugin do pass the client_id from route_thread_main onward. So when they are loaded, their client_id may be 2, while the main connected session has a client_id of 1. This causes the plugin to be loaded for each of them, which is why the plugin was being loaded twice. I'm not familiar enough with the zellij core to know which fix to pursue, but am listing some suggestions that came to mind below:
Load PluginInstruction in zellij-server/src/plugins/mod.rs to be more like Reload, where it doesn't pass the client_id on, i.e., passes a client_id of None. I tested this out locally and it works for me; the plugin load is only called once now when I run launch-or-focus-plugin. However, I'm not sure if this breaks other functionality, so I'm not sure if it is a good fix. For what it's worth, I used cargo xtask test to run the tests and none of them failed (though 70 were ignored, and many of these looked plugin related). I'm happy to push this as a PR to the zellij core if you think this is the correct fix. For reference, the git diff is:zellij binary works from within a running session so that it uses that session's route_thread_main rather than creating a new one. Alternatively, allow creating a new route_thread_main but make it pass the client ID of the already existent session. Again, not sure if this is a good idea, but might be a possible fix.leakec
commented
7 months ago @imsnif Just sending a gentle reminder so this doesn't get lost. No rush, on it as I know you are very busy. In the comment above I believe I've identified the issue and have a proposed fix to the Zellij core. If you like the proposed fix let me know and I'll submit it as a PR on the Zellij core.
leakec
commented
6 months ago A recent commit on the zellij core fixed this :)
imsnif
commented
6 months ago I am very happy to hear this @leakec - and you have my sincerest apologies for ignoring this issue for so long. I was honestly meaning to take a deeper look at it in the next week or two.
Thanks for all your hard work and for your patience!
leakec
commented
6 months ago No sweat @imsnif. I know you're super busy, and thank you for all you do for this amazing tool!
To re-create this issue:
Then run
twice. The first time everything should work as intended. The second time and all subsequent times, two instances of the multitask file will open in two different panes, i.e., two instances of the plugin are running. This persists even if I close and re-open a new
zellijsession.If I clear the
~/.cache/zellijfolder, then the plugin works as intended the first time I re-run it, but again opens two instances afterward.This is a new issue I'm only seeing with
zellij 0.39.0.