Skip to content

Commit

Permalink
fix: sync block execution failed due to Runtime mutably borrowed mo…
Browse files Browse the repository at this point in the history
…re than once (#2225)
  • Loading branch information
sxyazi committed Jan 19, 2025
1 parent ed3c9c4 commit 4c5698d
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 40 deletions.
15 changes: 11 additions & 4 deletions .github/workflows/draft.yml
Original file line number Diff line number Diff line change
Expand Up @@ -203,16 +203,22 @@ jobs:
runs-on: ubuntu-latest
needs: [build-unix, build-windows, build-musl, build-snap]
steps:
- uses: actions/download-artifact@v4
with:
merge-multiple: true

- run: |
echo 'NIGHTLY_BODY<<EOF' >> $GITHUB_ENV
echo "From commit: ${GITHUB_SHA:0:8}" >> $GITHUB_ENV
echo "Generated on: $(date -u +"%Y-%m-%d %H:%M") UTC" >> $GITHUB_ENV
echo "EOF" >> $GITHUB_ENV
- uses: actions/download-artifact@v4
with:
merge-multiple: true

- name: Update the tag
run: |
git config user.name "github-actions[bot]"
git config user.email "41898282+github-actions[bot]@users.noreply.github.com"
git tag --force nightly && git push --force origin nightly
- name: Nightly
uses: softprops/action-gh-release@v1
with:
Expand All @@ -223,3 +229,4 @@ jobs:
yazi-*.snap
name: Nightly Build
body: ${{ env.NIGHTLY_BODY }}
target_commitish: ${{ github.sha }}
6 changes: 3 additions & 3 deletions yazi-fm/src/app/commands/accept_payload.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use mlua::IntoLua;
use tracing::error;
use yazi_dds::{LOCAL, Payload, REMOTE};
use yazi_plugin::{LUA, RtRef};
use yazi_plugin::{LUA, RtRefMut};
use yazi_shared::event::CmdCow;

use crate::{app::App, lives::Lives};
Expand All @@ -27,11 +27,11 @@ impl App {
_ = Lives::scope(&self.cx, || {
let body = payload.body.into_lua(&LUA)?;
for (id, cb) in handlers {
LUA.named_registry_value::<RtRef>("rt")?.push(&id);
LUA.named_registry_value::<RtRefMut>("rt")?.push(&id);
if let Err(e) = cb.call::<()>(body.clone()) {
error!("Failed to run `{kind}` event handler in your `{id}` plugin: {e}");
}
LUA.named_registry_value::<RtRef>("rt")?.pop();
LUA.named_registry_value::<RtRefMut>("rt")?.pop();
}
Ok(())
});
Expand Down
14 changes: 8 additions & 6 deletions yazi-fm/src/app/commands/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ use std::fmt::Display;

use mlua::ObjectLike;
use scopeguard::defer;
use tracing::warn;
use tracing::{error, warn};
use yazi_dds::Sendable;
use yazi_plugin::{LUA, RtRef, loader::LOADER};
use yazi_plugin::{LUA, RtRefMut, loader::LOADER};
use yazi_proxy::{AppProxy, options::{PluginMode, PluginOpt}};

use crate::{app::App, lives::Lives};
Expand Down Expand Up @@ -50,26 +50,28 @@ impl App {
return self.cx.tasks.plugin_micro(opt);
}

match LUA.named_registry_value::<RtRef>("rt") {
match LUA.named_registry_value::<RtRefMut>("rt") {
Ok(mut r) => r.push(&opt.id),
Err(e) => return warn!("{e}"),
}
defer! { _ = LUA.named_registry_value::<RtRef>("rt").map(|mut r| r.pop()) }
defer! { _ = LUA.named_registry_value::<RtRefMut>("rt").map(|mut r| r.pop()) }

let plugin = match LOADER.load_with(&LUA, &opt.id, chunk) {
Ok(plugin) => plugin,
Err(e) => return warn!("{e}"),
};
drop(loader);

_ = Lives::scope(&self.cx, || {
let result = Lives::scope(&self.cx, || {
if let Some(cb) = opt.cb {
cb(&LUA, plugin)
} else {
let job = LUA.create_table_from([("args", Sendable::args_to_table(&LUA, opt.args)?)])?;

plugin.call_method("entry", job)
}
});
if let Err(e) = result {
error!("Sync plugin `{}` failed: {e}", opt.id);
}
}
}
2 changes: 1 addition & 1 deletion yazi-plugin/src/isolate/seek.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use yazi_shared::event::Cmd;

use crate::{bindings::Cast, elements::Rect, file::File};

pub fn seek_sync(cmd: &Cmd, file: yazi_fs::File, units: i16) {
pub fn seek_sync(cmd: &'static Cmd, file: yazi_fs::File, units: i16) {
let cb: PluginCallback = Box::new(move |lua, plugin| {
let job = lua.create_table_from([
("file", File::cast(lua, file)?.into_lua(lua)?),
Expand Down
18 changes: 9 additions & 9 deletions yazi-plugin/src/loader/require.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::sync::Arc;
use mlua::{ExternalResult, Function, IntoLua, Lua, MetaMethod, MultiValue, ObjectLike, Table, Value};

use super::LOADER;
use crate::RtRef;
use crate::RtRefMut;

pub(super) struct Require;

Expand All @@ -15,9 +15,9 @@ impl Require {
let s = &id.to_str()?;
futures::executor::block_on(LOADER.ensure(s)).into_lua_err()?;

lua.named_registry_value::<RtRef>("rt")?.push(s);
lua.named_registry_value::<RtRefMut>("rt")?.push(s);
let mod_ = LOADER.load(lua, s);
lua.named_registry_value::<RtRef>("rt")?.pop();
lua.named_registry_value::<RtRefMut>("rt")?.pop();

Self::create_mt(lua, s, mod_?, true)
})?,
Expand All @@ -31,9 +31,9 @@ impl Require {
let s = &id.to_str()?;
LOADER.ensure(s).await.into_lua_err()?;

lua.named_registry_value::<RtRef>("rt")?.push(s);
lua.named_registry_value::<RtRefMut>("rt")?.push(s);
let mod_ = LOADER.load(&lua, s);
lua.named_registry_value::<RtRef>("rt")?.pop();
lua.named_registry_value::<RtRefMut>("rt")?.pop();

Self::create_mt(&lua, s, mod_?, false)
})?,
Expand Down Expand Up @@ -73,19 +73,19 @@ impl Require {
if sync {
lua.create_function(move |lua, args: MultiValue| {
let (mod_, args) = Self::split_mod_and_args(lua, &id, args)?;
lua.named_registry_value::<RtRef>("rt")?.push(&id);
lua.named_registry_value::<RtRefMut>("rt")?.push(&id);
let result = mod_.call_function::<MultiValue>(&f, args);
lua.named_registry_value::<RtRef>("rt")?.pop();
lua.named_registry_value::<RtRefMut>("rt")?.pop();
result
})
} else {
lua.create_async_function(move |lua, args: MultiValue| {
let (id, f) = (id.clone(), f.clone());
async move {
let (mod_, args) = Self::split_mod_and_args(&lua, &id, args)?;
lua.named_registry_value::<RtRef>("rt")?.push(&id);
lua.named_registry_value::<RtRefMut>("rt")?.push(&id);
let result = mod_.call_async_function::<MultiValue>(&f, args).await;
lua.named_registry_value::<RtRef>("rt")?.pop();
lua.named_registry_value::<RtRefMut>("rt")?.pop();
result
}
})
Expand Down
8 changes: 6 additions & 2 deletions yazi-plugin/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,20 @@ use std::collections::{HashMap, VecDeque};

use mlua::{Function, UserData};

#[derive(Default)]
#[derive(Debug, Default)]
pub struct Runtime {
frames: VecDeque<RuntimeFrame>,
blocks: HashMap<String, Vec<Function>>,
}

#[derive(Debug)]
struct RuntimeFrame {
id: String,
calls: usize,
}

pub type RtRef = mlua::UserDataRefMut<Runtime>;
pub type RtRef = mlua::UserDataRef<Runtime>;
pub type RtRefMut = mlua::UserDataRefMut<Runtime>;

impl Runtime {
pub fn new(id: &str) -> Self {
Expand All @@ -31,6 +33,8 @@ impl Runtime {

pub fn current(&self) -> Option<&str> { self.frames.back().map(|f| f.id.as_str()) }

pub fn current_owned(&self) -> Option<String> { self.current().map(ToOwned::to_owned) }

pub fn next_block(&mut self) -> Option<usize> {
self.frames.back_mut().map(|f| {
f.calls += 1;
Expand Down
25 changes: 12 additions & 13 deletions yazi-plugin/src/utils/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,32 +6,31 @@ use yazi_proxy::{AppProxy, options::{PluginCallback, PluginOpt}};
use yazi_shared::event::Data;

use super::Utils;
use crate::{bindings::{MpscRx, MpscTx, MpscUnboundedRx, MpscUnboundedTx, OneshotRx, OneshotTx}, loader::LOADER, runtime::RtRef};
use crate::{RtRefMut, bindings::{MpscRx, MpscTx, MpscUnboundedRx, MpscUnboundedTx, OneshotRx, OneshotTx}, loader::LOADER, runtime::RtRef};

impl Utils {
pub(super) fn sync(lua: &Lua, isolate: bool) -> mlua::Result<Function> {
if isolate {
lua.create_function(|lua, ()| {
let Some(block) = lua.named_registry_value::<RtRef>("rt")?.next_block() else {
let Some(block) = lua.named_registry_value::<RtRefMut>("rt")?.next_block() else {
return Err("`ya.sync()` must be called in a plugin").into_lua_err();
};

lua.create_async_function(move |lua, args: MultiValue| async move {
if let Some(cur) = lua.named_registry_value::<RtRef>("rt")?.current() {
Sendable::list_to_values(&lua, Self::retrieve(cur, block, args).await?)
} else {
Err("block spawned by `ya.sync()` must be called in a plugin").into_lua_err()
}
let Some(cur) = lua.named_registry_value::<RtRef>("rt")?.current_owned() else {
return Err("block spawned by `ya.sync()` must be called in a plugin").into_lua_err();
};
Sendable::list_to_values(&lua, Self::retrieve(cur, block, args).await?)
})
})
} else {
lua.create_function(|lua, f: Function| {
let mut rt = lua.named_registry_value::<RtRef>("rt")?;
let mut rt = lua.named_registry_value::<RtRefMut>("rt")?;
if !rt.put_block(f.clone()) {
return Err("`ya.sync()` must be called in a plugin").into_lua_err();
}

let cur = rt.current().unwrap().to_owned();
let cur = rt.current_owned().unwrap();
lua.create_function(move |lua, mut args: MultiValue| {
args.push_front(Value::Table(LOADER.try_load(lua, &cur)?));
f.call::<MultiValue>(args)
Expand Down Expand Up @@ -78,14 +77,14 @@ impl Utils {
lua.create_async_function(|_lua, _futs: MultiValue| async move { Ok(()) })
}

async fn retrieve(id: &str, calls: usize, args: MultiValue) -> mlua::Result<Vec<Data>> {
async fn retrieve(id: String, calls: usize, args: MultiValue) -> mlua::Result<Vec<Data>> {
let args = Sendable::values_to_list(args)?;
let (tx, rx) = oneshot::channel::<Vec<Data>>();

let callback: PluginCallback = {
let id = id.to_owned();
let id_ = id.clone();
Box::new(move |lua, plugin| {
let Some(block) = lua.named_registry_value::<RtRef>("rt")?.get_block(&id, calls) else {
let Some(block) = lua.named_registry_value::<RtRef>("rt")?.get_block(&id_, calls) else {
return Err("sync block not found".into_lua_err());
};

Expand All @@ -99,7 +98,7 @@ impl Utils {
})
};

AppProxy::plugin(PluginOpt::new_callback(id, callback));
AppProxy::plugin(PluginOpt::new_callback(id.clone(), callback));

rx.await
.map_err(|_| format!("Failed to execute sync block-{calls} in `{id}` plugin").into_lua_err())
Expand Down
4 changes: 2 additions & 2 deletions yazi-proxy/src/options/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ impl Debug for PluginOpt {
}

impl PluginOpt {
pub fn new_callback(id: &str, cb: PluginCallback) -> Self {
Self { id: id.to_owned().into(), mode: PluginMode::Sync, cb: Some(cb), ..Default::default() }
pub fn new_callback(id: impl Into<Cow<'static, str>>, cb: PluginCallback) -> Self {
Self { id: id.into(), mode: PluginMode::Sync, cb: Some(cb), ..Default::default() }
}
}

Expand Down

0 comments on commit 4c5698d

Please sign in to comment.