Request for code review: PyO3 binding of tikv/raft-rs

jopemachine · July 3, 2023, 5:27am

Hello,

I am currently working on porting the raft-rs library, called rraft-py, to Python.
I also plan to give a presentation on this work at PyCon KR 2023 in August.

However, since I have limited experience with low-level programming and exposing Rust references in Python data structures, I believe it would be immensely helpful to receive advice from Rust experts in these areas.

Currently, I have written all the harness test codes from raft-rs in Python, and they are passing successfully. However, throughout several code reviews, I have discovered several mistakes such as memory leaks and incorrect usage of unsafe code leading to undefined behavior.

I would greatly appreciate any advice from Rust experts who have a solid understanding of these aspects.

I keep studying various smart pointers from pyo3 and Rust, but I feel many parts are still unclear.
Even a single piece of advice would be invaluable to me.

Thank you.

github.com

lablup/rraft-py/blob/main/src/utils/reference.rs

// Ref:
// RefMutContainer implementation is greatly inspired from huggingface/tokenizer's python binding's.
// https://github.com/huggingface/tokenizers/blob/main/bindings/python/src/utils/mod.rs
use pyo3::prelude::*;
use std::collections::HashMap;
use std::ops::{Deref, DerefMut};
use std::ptr::NonNull;
use std::sync::{Arc, Mutex, Weak};

use super::errors::{DestroyedRefUsedError, DESTROYED_ERR_MSG};

// References that need to be cleaned up.
type RefMutContainerTable<T> = HashMap<u64, Weak<Mutex<Option<NonNull<T>>>>>;

#[derive(Clone)]
pub struct RefMutOwner<T> {
    pub inner: T,
    refs: Arc<Mutex<RefMutContainerTable<T>>>,
}

This file has been truncated. show original

system · October 1, 2023, 5:27am

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.