Already Borrowed error trying to call a list of callbacks

[SamP20]

I'm trying to replicate the following code in Rust:

def step(self):
    # get event from queue
    ...

    callbacks = event.callbacks
    for callback in callbacks:
        callback(event)

I have tried the following, however the call1 is resulting in an Already Borrowed runtime error:

fn step(&mut self, py: Python<'_>) -> PyResult<()> {
    // Get entry from queue in self...
    ...

    let event = entry.0.event.as_ref(py);
    for cb in event.getattr("callbacks")?.iter()? {
        cb?.call1((event,))?;
    }

    ...
}

I'm at a bit of a loss as to how to debug and/or fix this besides trying random combinations of to_object and as_ref (I'm assuming to_object creates a new reference counted pointer to the original object). Has anyone got any suggestions?

[SamP20]

The issue was with the &mut self. One of the callbacks was calling another method on self (to insert into the event queue) which was resulting in the double borrow. I've replaced them with self_: &PyCell<Self> and ensured there's no outstanding borrows when the callbacks are called

[bkolligs]

@adamreichold i think the reason I’m seeing it is because I use py.allow_threads to release the GIL and spawn my rust threads. So the interpreter can pause a thread that may still have a mutable reference to self. I was able to fix it by just using some carefully placed Mutex’s and Cells and avoiding &mut self in my py methods block

[bkolligs]

This is what I was doing basically:

#[pyclass]
pub struct PanickyTest {
    progress: usize,
}

#[pymethods]
impl PanickyTest {
    #[new]
    pub fn new() -> Self {
        Self { progress: 0 }
    }

    pub fn start(&mut self, py: Python) {
        py.allow_threads(|| {
            for _ in 0..20 {
                std::thread::sleep(std::time::Duration::from_secs_f64(0.1));
                self.progress += 1;
            }
        })
    }

    pub fn get_progress(&mut self) -> usize {
        self.progress
    }
}

With the Python

import classmethods
import time
from threading import Thread

class T(Thread):

    def __init__(self):

        Thread.__init__(self)

        self._test = None

    def run(self):

        print("thread started")

        self._test = classmethods.PanickyTest()
        self._test.start()

        print("thread finished")

    def get_progress(self):
        if self._test != None:
            print(self._test.get_progress())
        else:
            print(None)

thread = T()
thread.start()

print("start")

tstart = time.time()
while time.time() - tstart < 10:
    time.sleep(1)
    thread.get_progress()

print("finish")

[adamreichold]

i think the reason I’m seeing it is because I use py.allow_threads to release the GIL and spawn my rust threads.

Indeed, I did not consider this. Yes, if you call into allow_threads, then another thread could also try to gain a borrow after it has acquired to GIL while your thread has released it.

One recommendation I would make for a type that is supposed to be used from multiple threads this way is to avoid PyCell altogether by marking it as #[pyclass(frozen)]. This means that you will only ever get access to &self in your #[pyfunction]s, but this will have the positive effect of avoiding such issues outright and also guide you towards using standard Rust means to implement synchronization, e.g. progress: AtomicUsize instead of progress: usize.

[bkolligs]

Ah that’s a good idea, let me experiment with that. Thanks!

[davidhewitt]

Agreed with all of the above, with the extra comment that it's not just allow_threads - if you call any Python code it's entirely possible the gil may be released in the same way. E.g. removing an element from a list might decrease recount to zero and cause a chain reaction of gc etc etc.

So it's best to assume that doing more or less anything can risk hitting this in multithreaded code.