fix: Ptr race conditions, add dev.toad.msg.ref.Message => toad_msg::Message conversion

This commit is contained in:
Orion Kindel 2023-04-11 17:42:44 -05:00
parent 8679ae0a78
commit c700a6c43c
Signed by untrusted user who does not match committer: orion
GPG Key ID: 6D4165AE4C928719
14 changed files with 362 additions and 121 deletions

View File

@ -40,13 +40,11 @@ impl Toad {
}
fn poll_req_impl(e: &mut java::Env, addr: i64) -> java::util::Optional<msg::ref_::Message> {
match unsafe {
Shared::deref::<Runtime>(/* TODO */ 0, addr).as_ref()
.unwrap()
}.poll_req()
{
match unsafe { Shared::deref::<Runtime>(addr).as_ref().unwrap() }.poll_req() {
| Ok(req) => {
let mr = msg::ref_::Message::new(e, req.unwrap().into());
let msg_ptr: *mut toad_msg::alloc::Message =
unsafe { Shared::alloc_message(req.unwrap().into()) };
let mr = msg::ref_::Message::new(e, msg_ptr.addr() as i64);
java::util::Optional::<msg::ref_::Message>::of(e, mr)
},
| Err(nb::Error::WouldBlock) => java::util::Optional::<msg::ref_::Message>::empty(e),

View File

@ -8,8 +8,15 @@ impl java::Class for Code {
}
impl Code {
pub fn new(e: &mut java::Env, code: toad_msg::Code) -> Self {
static CTOR: java::Constructor<Code, fn(i32, i32)> = java::Constructor::new();
pub fn from_toad(e: &mut java::Env, code: toad_msg::Code) -> Self {
static CTOR: java::Constructor<Code, fn(i16, i16)> = java::Constructor::new();
CTOR.invoke(e, code.class.into(), code.detail.into())
}
pub fn to_toad(&self, e: &mut java::Env) -> toad_msg::Code {
static CLASS: java::Method<Code, fn() -> i16> = java::Method::new("codeClass");
static DETAIL: java::Method<Code, fn() -> i16> = java::Method::new("codeDetail");
toad_msg::Code::new(CLASS.invoke(e, self) as u8, DETAIL.invoke(e, self) as u8)
}
}

View File

@ -0,0 +1,44 @@
use toad_jni::java;
use crate::dev::toad::ffi;
pub struct Id(java::lang::Object);
java::object_newtype!(Id);
impl java::Class for Id {
const PATH: &'static str = package!(dev.toad.msg.Id);
}
impl Id {
pub fn from_u16(e: &mut java::Env, id: u16) -> Self {
static CTOR: java::Constructor<Id, fn(i32)> = java::Constructor::new();
CTOR.invoke(e, id.into())
}
pub fn to_u16(&self, e: &mut java::Env) -> u16 {
static ID: java::Field<Id, ffi::u16> = java::Field::new("id");
ID.get(e, self).to_rust(e)
}
pub fn from_toad(e: &mut java::Env, toad: toad_msg::Id) -> Self {
Self::from_u16(e, toad.0)
}
pub fn to_toad(&self, e: &mut java::Env) -> toad_msg::Id {
toad_msg::Id(self.to_u16(e))
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn roundtrip() {
let mut e = crate::test::init();
let e = &mut e;
let toad = toad_msg::Id(444);
let toadj = Id::from_toad(e, toad);
assert_eq!(toadj.to_toad(e), toad);
}
}

View File

@ -5,3 +5,9 @@ pub use ty::Type;
mod code;
pub use code::Code;
mod id;
pub use id::Id;
mod token;
pub use token::Token;

View File

@ -1,9 +1,11 @@
use std::collections::BTreeMap;
use jni::objects::JClass;
use jni::sys::jobject;
use toad_jni::java::{self, Object};
use crate::dev::toad::msg::ref_::Opt;
use crate::dev::toad::msg::{Code, Type};
use crate::dev::toad::msg::{Code, Id, Token, Type};
use crate::mem::{Shared, SharedMemoryRegion};
pub struct Message(java::lang::Object);
@ -14,10 +16,29 @@ impl java::Class for Message {
}
impl Message {
pub fn new(env: &mut java::Env, message: toad_msg::alloc::Message) -> Self {
let ptr = unsafe { Shared::alloc_message(message) };
pub fn new(env: &mut java::Env, msg_addr: i64) -> Self {
static CTOR: java::Constructor<Message, fn(i64)> = java::Constructor::new();
CTOR.invoke(env, ptr.addr() as i64)
CTOR.invoke(env, msg_addr)
}
pub fn to_toad(&self, env: &mut java::Env) -> toad_msg::alloc::Message {
toad_msg::alloc::Message { ty: self.ty(env),
ver: toad_msg::Version::default(),
code: self.code(env),
id: self.id(env),
token: self.token(env),
payload: toad_msg::Payload(self.payload(env)),
opts: self.options(env)
.into_iter()
.map(|opt| {
(opt.number(env),
opt.values(env)
.into_iter()
.map(|v| toad_msg::OptValue(v.bytes(env)))
.collect())
})
.collect::<BTreeMap<toad_msg::OptNumber,
Vec<toad_msg::OptValue<Vec<u8>>>>>() }
}
pub fn close(&self, env: &mut java::Env) {
@ -30,9 +51,32 @@ impl Message {
TYPE.invoke(env, self).to_toad(env)
}
pub unsafe fn ptr<'a>(addr: i64) -> &'a mut toad_msg::alloc::Message {
crate::mem::Shared::deref::<toad_msg::alloc::Message>(/* TODO */ 0, addr).as_mut()
.unwrap()
pub fn id(&self, env: &mut java::Env) -> toad_msg::Id {
static ID: java::Method<Message, fn() -> Id> = java::Method::new("id");
ID.invoke(env, self).to_toad(env)
}
pub fn token(&self, env: &mut java::Env) -> toad_msg::Token {
static TOKEN: java::Method<Message, fn() -> Token> = java::Method::new("token");
TOKEN.invoke(env, self).to_toad(env)
}
pub fn code(&self, env: &mut java::Env) -> toad_msg::Code {
static CODE: java::Method<Message, fn() -> Code> = java::Method::new("code");
CODE.invoke(env, self).to_toad(env)
}
pub fn options(&self, env: &mut java::Env) -> Vec<Opt> {
static OPTIONS: java::Method<Message, fn() -> Vec<Opt>> = java::Method::new("optionRefs");
OPTIONS.invoke(env, self)
}
pub fn payload(&self, env: &mut java::Env) -> Vec<u8> {
static PAYLOAD: java::Method<Message, fn() -> Vec<i8>> = java::Method::new("payloadBytes");
PAYLOAD.invoke(env, self)
.into_iter()
.map(|i| u8::from_be_bytes(i.to_be_bytes()))
.collect()
}
}
@ -40,9 +84,13 @@ impl Message {
pub extern "system" fn Java_dev_toad_msg_ref_Message_id<'local>(mut env: java::Env<'local>,
_: JClass<'local>,
addr: i64)
-> i32 {
let msg = unsafe { Message::ptr(addr) };
msg.id.0 as i32
-> jobject {
let e = &mut env;
let msg = unsafe {
Shared::deref::<toad_msg::alloc::Message>(addr).as_ref()
.unwrap()
};
Id::from_toad(e, msg.id).yield_to_java(e)
}
#[no_mangle]
@ -50,8 +98,12 @@ pub extern "system" fn Java_dev_toad_msg_ref_Message_token<'local>(mut env: java
_: JClass<'local>,
addr: i64)
-> jobject {
let msg = unsafe { Message::ptr(addr) };
env.byte_array_from_slice(&msg.token.0).unwrap().as_raw()
let e = &mut env;
let msg = unsafe {
Shared::deref::<toad_msg::alloc::Message>(addr).as_ref()
.unwrap()
};
Token::from_toad(e, msg.token).yield_to_java(e)
}
#[no_mangle]
@ -59,7 +111,10 @@ pub extern "system" fn Java_dev_toad_msg_ref_Message_payload<'local>(mut env: ja
_: JClass<'local>,
addr: i64)
-> jobject {
let msg = unsafe { Message::ptr(addr) };
let msg = unsafe {
Shared::deref::<toad_msg::alloc::Message>(addr).as_ref()
.unwrap()
};
env.byte_array_from_slice(&msg.payload.0).unwrap().as_raw()
}
@ -68,7 +123,10 @@ pub extern "system" fn Java_dev_toad_msg_ref_Message_type<'local>(mut e: java::E
_: JClass<'local>,
addr: i64)
-> jobject {
let msg = unsafe { Message::ptr(addr) };
let msg = unsafe {
Shared::deref::<toad_msg::alloc::Message>(addr).as_ref()
.unwrap()
};
Type::new(&mut e, msg.ty).yield_to_java(&mut e)
}
@ -77,8 +135,11 @@ pub extern "system" fn Java_dev_toad_msg_ref_Message_code<'local>(mut e: java::E
_: JClass<'local>,
addr: i64)
-> jobject {
let msg = unsafe { Message::ptr(addr) };
Code::new(&mut e, msg.code).yield_to_java(&mut e)
let msg = unsafe {
Shared::deref::<toad_msg::alloc::Message>(addr).as_ref()
.unwrap()
};
Code::from_toad(&mut e, msg.code).yield_to_java(&mut e)
}
#[no_mangle]
@ -86,7 +147,10 @@ pub extern "system" fn Java_dev_toad_msg_ref_Message_opts<'local>(mut e: java::E
_: JClass<'local>,
addr: i64)
-> jobject {
let msg = unsafe { Message::ptr(addr) };
let msg = unsafe {
Shared::deref::<toad_msg::alloc::Message>(addr).as_ref()
.unwrap()
};
let opts = &msg.opts;
let refs = opts.into_iter()
@ -95,3 +159,68 @@ pub extern "system" fn Java_dev_toad_msg_ref_Message_opts<'local>(mut e: java::E
refs.yield_to_java(&mut e)
}
#[cfg(test)]
mod tests {
use toad_jni::java::Signature;
use toad_msg::{MessageOptions, Payload};
use super::*;
#[test]
fn roundtrip() {
let mut env = crate::test::init();
let e = &mut env;
let mut toad_msg = {
use tinyvec::array_vec;
use toad_msg::*;
Message::new(Type::Con, Code::GET, Id(333), Token(array_vec![1, 2, 3, 4]))
};
toad_msg.set_path("foo/bar/baz").ok();
toad_msg.set_payload(Payload(r#"{"id": 123, "stuff": ["abc"]}"#.as_bytes().to_vec()));
let ptr: *mut toad_msg::alloc::Message = Box::into_raw(Box::new(toad_msg));
let msg = Message::new(e, ptr.addr() as i64);
assert_eq!(&msg.to_toad(e), unsafe { ptr.as_ref().unwrap() });
}
#[test]
fn message_ref_should_throw_when_used_after_close() {
let mut env = crate::test::init();
let e = &mut env;
let mut toad_msg = {
use tinyvec::array_vec;
use toad_msg::*;
Message::new(Type::Con, Code::GET, Id(333), Token(array_vec![1, 2, 3, 4]))
};
toad_msg.set_path("foo/bar/baz").ok();
toad_msg.set_payload(Payload(r#"{"id": 123, "stuff": ["abc"]}"#.as_bytes().to_vec()));
let ptr: *mut toad_msg::alloc::Message = Box::into_raw(Box::new(toad_msg));
let msg = Message::new(e, ptr.addr() as i64);
assert_eq!(msg.ty(e), toad_msg::Type::Con);
msg.close(e);
let msg_o = msg.downcast(e);
e.call_method(msg_o.as_local(),
"type",
Signature::of::<fn() -> Type>(),
&[])
.ok();
let err = e.exception_occurred().unwrap();
e.exception_clear().unwrap();
assert!(e.is_instance_of(err, concat!(package!(dev.toad.ffi.Ptr), "$ExpiredError"))
.unwrap());
}
}

View File

@ -5,7 +5,7 @@ use toad_jni::java::{self, Object};
use toad_msg::OptNumber;
use super::OptValue;
use crate::mem::SharedMemoryRegion;
use crate::mem::{Shared, SharedMemoryRegion};
pub struct Opt(pub java::lang::Object);
@ -26,28 +26,31 @@ impl Opt {
OptNumber(NUMBER.get(env, self) as u32)
}
pub unsafe fn values_ptr<'a>(addr: i64) -> &'a mut Vec<toad_msg::OptValue<Vec<u8>>> {
crate::mem::Shared::deref::<Vec<toad_msg::OptValue<Vec<u8>>>>(/* TODO */ 0, addr).as_mut()
.unwrap()
pub fn values(&self, env: &mut java::Env) -> Vec<OptValue> {
static VALUES: java::Method<Opt, fn() -> Vec<OptValue>> = java::Method::new("valueRefs");
VALUES.invoke(env, self)
}
}
#[no_mangle]
pub extern "system" fn Java_dev_toad_msg_Option_number<'local>(mut env: JNIEnv<'local>,
o: JObject<'local>,
p: i64)
-> i64 {
pub extern "system" fn Java_dev_toad_msg_ref_Option_number<'local>(mut env: JNIEnv<'local>,
o: JObject<'local>,
p: i64)
-> i64 {
java::lang::Object::from_local(&mut env, o).upcast_to::<Opt>(&mut env)
.number(&mut env)
.0 as i64
}
#[no_mangle]
pub extern "system" fn Java_dev_toad_msg_Option_values<'local>(mut e: JNIEnv<'local>,
_: JClass<'local>,
p: i64)
-> jobject {
let o = &unsafe { Opt::values_ptr(p) };
pub extern "system" fn Java_dev_toad_msg_ref_Option_values<'local>(mut e: JNIEnv<'local>,
_: JClass<'local>,
p: i64)
-> jobject {
let o = unsafe {
Shared::deref::<Vec<toad_msg::OptValue<Vec<u8>>>>(p).as_ref()
.unwrap()
};
let refs = o.iter()
.map(|v| OptValue::new(&mut e, (&v.0 as *const Vec<u8>).addr() as i64))

View File

@ -2,7 +2,7 @@ use jni::objects::JClass;
use jni::sys::jobject;
use toad_jni::java;
use crate::mem::SharedMemoryRegion;
use crate::mem::{Shared, SharedMemoryRegion};
pub struct OptValue(java::lang::Object);
@ -17,9 +17,12 @@ impl OptValue {
CTOR.invoke(env, addr)
}
pub unsafe fn ptr<'a>(addr: i64) -> &'a mut toad_msg::OptValue<Vec<u8>> {
crate::mem::Shared::deref::<toad_msg::OptValue<Vec<u8>>>(/* TODO */ 0, addr).as_mut()
.unwrap()
pub fn bytes(&self, env: &mut java::Env) -> Vec<u8> {
static AS_BYTES: java::Method<OptValue, fn() -> Vec<i8>> = java::Method::new("asBytes");
AS_BYTES.invoke(env, self)
.into_iter()
.map(|i| u8::from_be_bytes(i.to_be_bytes()))
.collect()
}
}
@ -28,6 +31,9 @@ pub extern "system" fn Java_dev_toad_msg_ref_OptionValue_bytes<'local>(mut env:
_: JClass<'local>,
p: i64)
-> jobject {
let val = unsafe { OptValue::ptr(p) };
let val = unsafe {
Shared::deref::<toad_msg::OptValue<Vec<u8>>>(p).as_ref()
.unwrap()
};
env.byte_array_from_slice(val.as_bytes()).unwrap().as_raw()
}

View File

@ -0,0 +1,54 @@
use tinyvec::ArrayVec;
use toad_jni::java;
use crate::dev::toad::ffi;
pub struct Token(java::lang::Object);
java::object_newtype!(Token);
impl java::Class for Token {
const PATH: &'static str = package!(dev.toad.msg.Token);
}
impl Token {
pub fn from_bytes(e: &mut java::Env, bytes: &[u8]) -> Self {
static CTOR: java::Constructor<Token, fn(Vec<i8>)> = java::Constructor::new();
CTOR.invoke(e,
bytes.iter()
.copied()
.map(|u| i8::from_be_bytes(u.to_be_bytes()))
.collect())
}
pub fn to_bytes(&self, e: &mut java::Env) -> ArrayVec<[u8; 8]> {
static BYTES: java::Field<Token, Vec<i8>> = java::Field::new("bytes");
BYTES.get(e, self)
.into_iter()
.map(|i| u8::from_be_bytes(i.to_be_bytes()))
.collect()
}
pub fn from_toad(e: &mut java::Env, token: toad_msg::Token) -> Self {
Self::from_bytes(e, &token.0)
}
pub fn to_toad(&self, e: &mut java::Env) -> toad_msg::Token {
toad_msg::Token(self.to_bytes(e))
}
}
#[cfg(test)]
mod tests {
use tinyvec::array_vec;
use super::*;
#[test]
fn roundtrip() {
let mut e = crate::test::init();
let e = &mut e;
let toad = toad_msg::Token(array_vec![1, 2, 3, 4, 5]);
let toadj = Token::from_toad(e, toad);
assert_eq!(toadj.to_toad(e), toad);
}
}

View File

@ -46,31 +46,8 @@ fn runtime_poll_req(State { runtime,
assert!(runtime.poll_req(env).is_some());
}
fn message_ref_should_throw_when_used_after_close(State {runtime, env, client, srv_addr, ..}: &mut State)
{
let request = Message::new(Type::Con, Code::GET, Id(0), Token(Default::default()));
client.send_msg(Addrd(request, *srv_addr)).unwrap();
let req = runtime.poll_req(env).unwrap();
assert_eq!(req.ty(env), Type::Con);
req.close(env);
let req_o = req.downcast(env);
env.call_method(req_o.as_local(),
"type",
Signature::of::<fn() -> dev::toad::msg::Type>(),
&[])
.ok();
let err = env.exception_occurred().unwrap();
env.exception_clear().unwrap();
assert!(env.is_instance_of(err, concat!(package!(dev.toad.ffi.Ptr), "$ExpiredError"))
.unwrap());
}
#[test]
fn e2e_test_suite() {
let mut state = init();
runtime_poll_req(&mut state);
message_ref_should_throw_when_used_after_close(&mut state);
}

View File

@ -5,8 +5,13 @@ use std::ffi::c_void;
use jni::JavaVM;
use mem::SharedMemoryRegion;
pub type Runtime = ::toad::std::Platform<::toad::std::dtls::N,
::toad::step::runtime::std::Runtime<::toad::std::dtls::N>>;
mod runtime {
use toad::std::{dtls, Platform};
use toad::step::runtime::std::Runtime as DefaultSteps;
pub type Runtime = Platform<dtls::N, DefaultSteps<dtls::N>>;
}
pub use runtime::Runtime;
#[macro_export]
macro_rules! package {

View File

@ -23,20 +23,21 @@ pub trait SharedMemoryRegion: core::default::Default + core::fmt::Debug + Copy {
/// Teardown
unsafe fn dealloc();
unsafe fn shared_region(addr: i64) -> *mut u8;
unsafe fn shared_region() -> *mut u8;
/// Coerce a `long` rep of a pointer to some data within the
/// shared memory region.
unsafe fn deref<T>(shared_region_addr: i64, addr: i64) -> *mut T {
Self::shared_region(shared_region_addr).with_addr(addr as usize)
.cast::<T>()
unsafe fn deref<T>(addr: i64) -> *mut T {
Self::shared_region().with_addr(addr as usize).cast::<T>()
}
}
static mut MEM: *mut Mem = core::ptr::null_mut();
static mut MEM: Mem = Mem { runtime: None,
messages: vec![],
messages_lock: Mutex::new(()) };
struct Mem {
runtime: crate::Runtime,
runtime: Option<crate::Runtime>,
messages: Vec<Message>,
/// Lock used by `alloc_message` and `dealloc_message` to ensure
@ -52,39 +53,40 @@ struct Mem {
#[derive(Default, Debug, Clone, Copy)]
pub struct GlobalStatic;
impl SharedMemoryRegion for GlobalStatic {
unsafe fn dealloc() {
if !MEM.is_null() {
drop(Box::from_raw(MEM));
}
}
unsafe fn dealloc() {}
unsafe fn init(r: impl FnOnce() -> crate::Runtime) -> *mut crate::Runtime {
if MEM.is_null() {
MEM = Box::into_raw(Box::new(Mem { runtime: r(),
messages: vec![],
messages_lock: Mutex::new(()) }));
if MEM.runtime.is_none() {
MEM.runtime = Some(r());
}
&mut (*MEM).runtime as _
MEM.runtime.as_mut().unwrap() as _
}
unsafe fn alloc_message(m: Message) -> *mut Message {
let _lock = (*MEM).messages_lock.lock();
(*MEM).messages.push(m);
&mut (*MEM).messages[(*MEM).messages.len() - 1] as _
let Mem { ref mut messages,
ref mut messages_lock,
.. } = &mut MEM;
let _lock = messages_lock.lock();
messages.push(m);
let len = messages.len();
&mut messages[len - 1] as _
}
unsafe fn dealloc_message(m: *mut Message) {
let _lock = (*MEM).messages_lock.lock();
let ix = m.offset_from((*MEM).messages.as_slice().as_ptr());
let Mem { messages,
messages_lock,
.. } = &mut MEM;
let _lock = messages_lock.lock();
let ix = m.offset_from(messages.as_slice().as_ptr());
if ix.is_negative() {
panic!()
}
(*MEM).messages.remove(ix as usize);
messages.remove(ix as usize);
}
unsafe fn shared_region(_: i64) -> *mut u8 {
MEM as _
unsafe fn shared_region() -> *mut u8 {
&mut MEM as *mut Mem as *mut u8
}
}

View File

@ -18,7 +18,7 @@ import java.util.Optional;
*/
public class Ptr {
private static final HashSet<Long> validAddresses = new HashSet<>();
private static volatile HashSet<Long> validAddresses = new HashSet<>();
protected final long addr;
private final String clazz;
@ -27,7 +27,7 @@ public class Ptr {
/**
* Associate a class instance with a native pointer
*/
public static Ptr register(Class c, long addr) {
public static synchronized Ptr register(Class c, long addr) {
var trace = Thread.currentThread().getStackTrace();
var traceStr = Arrays
.asList(trace)
@ -49,7 +49,7 @@ public class Ptr {
/**
* Invokes the cleaning action on the object associated with an address
*/
public void release() {
public synchronized void release() {
Ptr.validAddresses.remove(this.addr);
}
@ -57,13 +57,13 @@ public class Ptr {
* Throw `ExpiredError` if object has been leaked
* outside of its appropriate context.
*/
public void ensureValid() {
public synchronized void ensureValid() {
if (!Ptr.validAddresses.contains(this.addr)) {
throw new ExpiredError(this);
}
}
public long addr() {
public synchronized long addr() {
this.ensureValid();
return this.addr;
}

View File

@ -1,44 +1,52 @@
package dev.toad.msg;
import dev.toad.ffi.u8;
public class Code {
private final int clazz;
private final int detail;
final u8 clazz;
final u8 detail;
public Code(int clazz, int detail) {
this.clazz = clazz;
this.detail = detail;
public Code(short clazz, short detail) {
this.clazz = new u8(clazz);
this.detail = new u8(detail);
}
public short codeClass() {
return this.clazz.shortValue();
}
public short codeDetail() {
return this.detail.shortValue();
}
@Override
public String toString() {
if (this.isRequest()) {
switch (this.detail) {
case 1:
return "GET";
case 2:
return "PUT";
case 3:
return "POST";
case 4:
return "DELETE";
default:
throw new Error();
}
return switch ((Short) this.detail.shortValue()) {
case 1 -> "GET";
case 2 -> "PUT";
case 3 -> "POST";
case Short other -> "DELETE";
};
} else {
return String.format("%d.%d", this.clazz, this.detail);
return String.format(
"%d.%d",
this.clazz.shortValue(),
this.detail.shortValue()
);
}
}
public boolean isRequest() {
return this.clazz == 0 && this.detail > 0;
return this.codeClass() == 0 && this.codeDetail() > 0;
}
public boolean isResponse() {
return this.clazz > 1;
return this.codeClass() > 1;
}
public boolean isEmpty() {
return this.clazz == 0 && this.detail == 0;
return this.codeClass() == 0 && this.codeDetail() == 0;
}
}

View File

@ -1,14 +1,16 @@
package dev.toad.msg;
import dev.toad.ffi.u16;
public final class Id {
final int id;
final u16 id;
public Id(int id) {
this.id = id;
this.id = new u16(id);
}
public int toInt() {
return this.id;
return this.id.intValue();
}
}