Skip to content

Commit 4e2afd6

Browse files
author
Gemini Agent
committed
Refactor FVM reservations to use explicit machine instance
Removes the CURRENT_MACHINE global in Rust and passes the executor pointer explicitly through CGO to Rust. This prevents safety issues with concurrent FVM instances.
1 parent 9416272 commit 4e2afd6

4 files changed

Lines changed: 17 additions & 97 deletions

File tree

cgo/fvm.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ package cgo
2020
// 6 = ErrPlanTooLarge
2121
// 7 = ErrOverflow
2222
// 8 = ErrReservationInvariant
23-
int32_t FVM_BeginReservations(const uint8_t *cbor_plan_ptr, size_t cbor_plan_len, const uint8_t **error_msg_ptr, size_t *error_msg_len);
24-
int32_t FVM_EndReservations(const uint8_t **error_msg_ptr, size_t *error_msg_len);
2523
void FVM_DestroyReservationErrorMessage(uint8_t *error_msg_ptr, size_t error_msg_len);
2624
*/
2725
import "C"
@@ -115,11 +113,11 @@ func FvmMachineFlush(executor *FvmMachine) ([]byte, error) {
115113
// FvmBeginReservations invokes the FVM_BeginReservations C ABI with a CBOR-encoded plan.
116114
// It returns the raw reservation status code as defined by FvmReservationStatus,
117115
// along with an optional, human-readable error message from the engine.
118-
func FvmBeginReservations(plan SliceRefUint8) (int32, string) {
116+
func FvmBeginReservations(executor *FvmMachine, plan SliceRefUint8) (int32, string) {
119117
var msgPtr *C.uint8_t
120118
var msgLen C.size_t
121119

122-
status := C.FVM_BeginReservations(plan.ptr, plan.len, &msgPtr, &msgLen)
120+
status := C.FVM_BeginReservations((*C.InnerFvmMachine_t)(executor), plan.ptr, plan.len, &msgPtr, &msgLen)
123121

124122
if msgPtr == nil || msgLen == 0 {
125123
return int32(status), ""
@@ -132,11 +130,11 @@ func FvmBeginReservations(plan SliceRefUint8) (int32, string) {
132130
}
133131

134132
// FvmEndReservations invokes the FVM_EndReservations C ABI and returns the raw status code.
135-
func FvmEndReservations() (int32, string) {
133+
func FvmEndReservations(executor *FvmMachine) (int32, string) {
136134
var msgPtr *C.uint8_t
137135
var msgLen C.size_t
138136

139-
status := C.FVM_EndReservations(&msgPtr, &msgLen)
137+
status := C.FVM_EndReservations((*C.InnerFvmMachine_t)(executor), &msgPtr, &msgLen)
140138

141139
if msgPtr == nil || msgLen == 0 {
142140
return int32(status), ""

fvm.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ func CreateFVM(opts *FVMOpts) (*FVM, error) {
160160
// optionally wrapped with a short human-readable message from the engine.
161161
func (f *FVM) BeginReservations(plan []byte) error {
162162
defer runtime.KeepAlive(f)
163-
status, msg := cgo.FvmBeginReservations(cgo.AsSliceRefUint8(plan))
163+
status, msg := cgo.FvmBeginReservations(f.executor, cgo.AsSliceRefUint8(plan))
164164
baseErr := ReservationStatusToError(status)
165165
if baseErr == nil {
166166
return nil
@@ -176,7 +176,7 @@ func (f *FVM) BeginReservations(plan []byte) error {
176176
// optionally wrapped with a short human-readable message from the engine.
177177
func (f *FVM) EndReservations() error {
178178
defer runtime.KeepAlive(f)
179-
status, msg := cgo.FvmEndReservations()
179+
status, msg := cgo.FvmEndReservations(f.executor)
180180
baseErr := ReservationStatusToError(status)
181181
if baseErr == nil {
182182
return nil

rust/Cargo.lock

Lines changed: 7 additions & 16 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

rust/src/fvm/machine.rs

Lines changed: 4 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -31,19 +31,6 @@ use crate::util::types::{catch_panic_response, catch_panic_response_no_default,
3131

3232
const STACK_SIZE: usize = 64 << 20; // 64MiB
3333

34-
#[derive(Copy, Clone)]
35-
struct MachinePtr(*const InnerFvmMachine);
36-
37-
// Safety: MachinePtr is only read or written behind a mutex and points to an
38-
// InnerFvmMachine that is itself synchronized internally.
39-
unsafe impl Send for MachinePtr {}
40-
41-
impl Default for MachinePtr {
42-
fn default() -> Self {
43-
MachinePtr(std::ptr::null())
44-
}
45-
}
46-
4734
lazy_static! {
4835
static ref CONCURRENCY: u32 = get_concurrency();
4936
static ref ENGINES: MultiEngineContainer = MultiEngineContainer::with_concurrency(*CONCURRENCY);
@@ -53,8 +40,6 @@ lazy_static! {
5340
.prefix("fvm")
5441
.stack_size(STACK_SIZE)
5542
);
56-
static ref CURRENT_MACHINE: std::sync::Mutex<MachinePtr> =
57-
std::sync::Mutex::new(MachinePtr::default());
5843
}
5944

6045
const LOTUS_FVM_CONCURRENCY_ENV_NAME: &str = "LOTUS_FVM_CONCURRENCY";
@@ -178,14 +163,6 @@ fn create_fvm_machine_generic(
178163
let inner = engine.new_executor(config, blockstore, externs)?;
179164
let boxed: repr_c::Box<InnerFvmMachine> = Box::new(inner).into();
180165

181-
// Track the most recently created machine for reservation sessions.
182-
{
183-
let mut current = CURRENT_MACHINE
184-
.lock()
185-
.map_err(|e| anyhow!("current executor lock poisoned: {e}"))?;
186-
*current = MachinePtr(&*boxed as *const InnerFvmMachine);
187-
}
188-
189166
Ok(Some(boxed))
190167
})
191168
}
@@ -612,6 +589,7 @@ fn map_reservation_error_to_status(
612589
#[ffi_export]
613590
#[allow(non_snake_case)]
614591
fn FVM_BeginReservations(
592+
executor: &'_ InnerFvmMachine,
615593
cbor_plan_ptr: *const u8,
616594
cbor_plan_len: usize,
617595
error_msg_ptr_out: *mut *const u8,
@@ -659,31 +637,7 @@ fn FVM_BeginReservations(
659637
}
660638
};
661639

662-
let machine_ptr = match CURRENT_MACHINE.lock() {
663-
Ok(guard) => guard.0,
664-
Err(_) => {
665-
set_reservation_error_message_out(
666-
error_msg_ptr_out,
667-
error_msg_len_out,
668-
"reservation invariant violated: current executor lock poisoned",
669-
);
670-
return FvmReservationStatus::ErrReservationInvariant;
671-
}
672-
};
673-
674-
if machine_ptr.is_null() {
675-
set_reservation_error_message_out(
676-
error_msg_ptr_out,
677-
error_msg_len_out,
678-
"reservations not implemented for current machine",
679-
);
680-
return FvmReservationStatus::ErrNotImplemented;
681-
}
682-
683-
// SAFETY: the pointer is set when the machine is created and
684-
// is expected to remain valid while reservations are used.
685-
let inner = unsafe { &*machine_ptr };
686-
let machine_mutex = match &inner.machine {
640+
let machine_mutex = match &executor.machine {
687641
Some(m) => m,
688642
None => {
689643
set_reservation_error_message_out(
@@ -716,36 +670,13 @@ fn FVM_BeginReservations(
716670
#[ffi_export]
717671
#[allow(non_snake_case)]
718672
fn FVM_EndReservations(
673+
executor: &'_ InnerFvmMachine,
719674
error_msg_ptr_out: *mut *const u8,
720675
error_msg_len_out: *mut usize,
721676
) -> FvmReservationStatus {
722677
clear_reservation_error_message_out(error_msg_ptr_out, error_msg_len_out);
723678

724-
let machine_ptr = match CURRENT_MACHINE.lock() {
725-
Ok(guard) => guard.0,
726-
Err(_) => {
727-
set_reservation_error_message_out(
728-
error_msg_ptr_out,
729-
error_msg_len_out,
730-
"reservation invariant violated: current executor lock poisoned",
731-
);
732-
return FvmReservationStatus::ErrReservationInvariant;
733-
}
734-
};
735-
736-
if machine_ptr.is_null() {
737-
set_reservation_error_message_out(
738-
error_msg_ptr_out,
739-
error_msg_len_out,
740-
"reservations not implemented for current machine",
741-
);
742-
return FvmReservationStatus::ErrNotImplemented;
743-
}
744-
745-
// SAFETY: the pointer is set when the machine is created and
746-
// is expected to remain valid while reservations are used.
747-
let inner = unsafe { &*machine_ptr };
748-
let machine_mutex = match &inner.machine {
679+
let machine_mutex = match &executor.machine {
749680
Some(m) => m,
750681
None => {
751682
set_reservation_error_message_out(

0 commit comments

Comments
 (0)