Skip to content

Commit

Permalink
Disallow processes sending messages to themselves
Browse files Browse the repository at this point in the history 
Processes sending messages to themselves can result in deadlocks,
whether the message's result is awaited immediately or not. Instead of
trying to go through all sorts of hoops to detect this as late as
possible, we'll just check for it when sending messages.

This fixes https://gitlab.com/inko-lang/inko/-/issues/259.

Changelog: fixed
  • Loading branch information
@yorickpeterse
yorickpeterse committed Sep 20, 2022
1 parent 9dd29cd commit d496aea
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 42 deletions.
28 changes: 22 additions & 6 deletions vm/src/instructions/process.rs
View file
Expand Up @@ -11,6 +11,9 @@ use crate::scheduler::timeouts::Timeout;
use crate::state::State;
use std::time::Duration;

const SEND_ERROR: &str = "Processes can't send messages to themselves, \
as this could result in deadlocks";

#[inline(always)]
pub(crate) fn allocate(state: &State, class_idx: u32) -> Pointer {
let class_index = ClassIndex::new(class_idx);
Expand All @@ -29,19 +32,24 @@ pub(crate) fn send_message(
receiver_ptr: Pointer,
method: u16,
wait: bool,
) -> bool {
) -> Result<bool, String> {
let mut receiver = unsafe { ProcessPointer::from_pointer(receiver_ptr) };

if sender == receiver {
return Err(SEND_ERROR.to_string());
}

let args = task.take_arguments();

match receiver.send_message(MethodIndex::new(method), sender, args, wait) {
RescheduleRights::AcquiredWithTimeout => {
state.timeout_worker.increase_expired_timeouts();
}
RescheduleRights::Acquired => {}
_ => return false,
_ => return Ok(false),
}

if wait {
let switch = if wait {
// When awaiting the result immediately we want to keep latency as small
// as possible. To achieve this we reschedule the receiver (if allowed)
// onto the current worker with a high priority.
Expand All @@ -50,18 +58,26 @@ pub(crate) fn send_message(
} else {
thread.schedule(receiver);
false
}
};

Ok(switch)
}

#[inline(always)]
pub(crate) fn send_async_message(
state: &State,
thread: &mut Thread,
sender: ProcessPointer,
mut task: TaskPointer,
receiver_ptr: Pointer,
method: u16,
) -> Pointer {
) -> Result<Pointer, String> {
let mut receiver = unsafe { ProcessPointer::from_pointer(receiver_ptr) };

if sender == receiver {
return Err(SEND_ERROR.to_string());
}

let fut_state = FutureState::new();
let fut =
Future::alloc(state.permanent_space.future_class(), fut_state.clone());
Expand All @@ -79,7 +95,7 @@ pub(crate) fn send_async_message(
_ => {}
}

fut
Ok(fut)
}

#[inline(always)]
Expand Down
52 changes: 16 additions & 36 deletions vm/src/machine.rs
View file
Expand Up @@ -27,24 +27,6 @@ macro_rules! reset {
}};
}

/// Handles a function that may produce an Inko panic.
macro_rules! try_panic {
($expr: expr, $loop_state: expr) => {{
match $expr {
Ok(thing) => thing,
Err(msg) => vm_panic!(msg, $loop_state),
}
}};
}

macro_rules! vm_panic {
($message: expr, $loop_state: expr) => {{
$loop_state.rewind();

return Err($message);
}};
}

/// The state of an interpreter loop, such as the context being executed.
///
/// We use a structure here so we don't need to pass around tons of variables
Expand Down Expand Up @@ -229,40 +211,39 @@ impl<'a> Machine<'a> {
let reg = ins.arg(0);
let a = state.context.get_register(ins.arg(1));
let b = state.context.get_register(ins.arg(2));
let res = try_panic!(integer::add(self.state, a, b), state);
let res = integer::add(self.state, a, b)?;

state.context.set_register(reg, res);
}
Opcode::IntDiv => {
let reg = ins.arg(0);
let a = state.context.get_register(ins.arg(1));
let b = state.context.get_register(ins.arg(2));
let res = try_panic!(integer::div(self.state, a, b), state);
let res = integer::div(self.state, a, b)?;

state.context.set_register(reg, res);
}
Opcode::IntMul => {
let reg = ins.arg(0);
let a = state.context.get_register(ins.arg(1));
let b = state.context.get_register(ins.arg(2));
let res = try_panic!(integer::mul(self.state, a, b), state);
let res = integer::mul(self.state, a, b)?;

state.context.set_register(reg, res);
}
Opcode::IntSub => {
let reg = ins.arg(0);
let a = state.context.get_register(ins.arg(1));
let b = state.context.get_register(ins.arg(2));
let res = try_panic!(integer::sub(self.state, a, b), state);
let res = integer::sub(self.state, a, b)?;

state.context.set_register(reg, res);
}
Opcode::IntMod => {
let reg = ins.arg(0);
let a = state.context.get_register(ins.arg(1));
let b = state.context.get_register(ins.arg(2));
let res =
try_panic!(integer::modulo(self.state, a, b), state);
let res = integer::modulo(self.state, a, b)?;

state.context.set_register(reg, res);
}
Expand Down Expand Up @@ -294,15 +275,15 @@ impl<'a> Machine<'a> {
let reg = ins.arg(0);
let a = state.context.get_register(ins.arg(1));
let b = state.context.get_register(ins.arg(2));
let res = try_panic!(integer::shl(self.state, a, b), state);
let res = integer::shl(self.state, a, b)?;

state.context.set_register(reg, res);
}
Opcode::IntShr => {
let reg = ins.arg(0);
let a = state.context.get_register(ins.arg(1));
let b = state.context.get_register(ins.arg(2));
let res = try_panic!(integer::shr(self.state, a, b), state);
let res = integer::shr(self.state, a, b)?;

state.context.set_register(reg, res);
}
Expand Down Expand Up @@ -524,7 +505,7 @@ impl<'a> Machine<'a> {

let switch = process::send_message(
self.state, thread, task, process, rec, method, wait,
);
)?;

if switch {
return Ok(());
Expand All @@ -535,8 +516,8 @@ impl<'a> Machine<'a> {
let rec = state.context.get_register(ins.arg(1));
let method = ins.arg(2);
let res = process::send_async_message(
self.state, thread, task, rec, method,
);
self.state, thread, process, task, rec, method,
)?;

state.context.set_register(reg, res);
}
Expand Down Expand Up @@ -622,10 +603,9 @@ impl<'a> Machine<'a> {
Opcode::Panic => {
let msg = state.context.get_register(ins.arg(0));

vm_panic!(
unsafe { InkoString::read(&msg).to_string() },
state
);
state.rewind();

return Err(unsafe { InkoString::read(&msg).to_string() });
}
Opcode::Exit => {
let status = state.context.get_register(ins.arg(0));
Expand Down Expand Up @@ -741,9 +721,9 @@ impl<'a> Machine<'a> {
process.set_return_value(val)
}
Err(RuntimeError::Panic(msg)) => {
state.save();
task.clear_arguments();
vm_panic!(msg, state);

return Err(msg);
}
Err(RuntimeError::Error(value)) => {
state.save();
Expand Down Expand Up @@ -846,7 +826,7 @@ impl<'a> Machine<'a> {
Opcode::CheckRefs => {
let ptr = state.context.get_register(ins.arg(0));

try_panic!(general::check_refs(ptr), state);
general::check_refs(ptr)?;
}
Opcode::Free => {
let obj = state.context.get_register(ins.arg(0));
Expand Down

0 comments on commit d496aea

Please sign in to comment.