Debugging closed channel and ManuallyDrop

[siscia]

Folks,

I have an issue and I would like to share it with you, most likely my understading of things is a little of.

I have an FFI pointer that I populate with a struct that looks like this.

struct DBKey {
    tx: ManuallyDrop<Sender<Stuff>>
    ... other_stuff ...
}

On a separated thread I got a loop that looks like the following.

for message in rx.iter() {
    ... do_stuff ...
}
log("out of the loop")

The sender in the struct is linked to the receiver in the loop.

The loop is suppose to be an infinite loop, it should just consume messages as they arrive, one by one. Unfortunately it does not and it terminate logging the "out of loop" message.

I obtain the structure with code that looks like this:

let ptr = unsafe { ... FFI call ...} as *mut DBKey; // get raw pointer
let dbkey = std::ptr::read(ptr); // read the raw pointer
let dbkey = ManuallyDrop::new(dbkey) // avoid to drop the structure

And I obtain the sender like this:

... continuing from above...
let dbkey : ManuallyDrop<DBKey> = { ... function above ... }
Ok(dbkey.tx.clone()) // clone the Sender wrapped in the ManuallyDrop above

I don't understand what is going on.

I log the DBKey drop, and it is not happening. So I never drop the main holder of the Receiver.

However the "infinite loop" terminates, so I guess that somehow all the Senders are dropped, but how?

Can somebody shade some light into this? Or if there are ny ideas on how to debug this kind of problem.

[matklad]

I would carefully scrutinize the unsafe code. Seeing ptr::read, a plausible explanation is that something else drops that memory?

[siscia]

Hummm, possible but unlikely.

Different unsafe code (that does roughly the same thing) is working since months in production. This was an attempt to remove that unsafeness.

[siscia]

So just to explain better the problem.

This is the version before the refactoring that works.

    pub fn get_channel(        
        &self,
    ) -> Result<ManuallyDrop<Sender<Command>>, RediSQLError> where {
            match self.key_type() {
            KeyTypes::RediSQL => unsafe {
                let dbkey = {  
                    rm::ffi::RedisModule_ModuleTypeGetValue.unwrap()(
                        self.key,
                    ) as *mut DBKey
                };             
                Ok((*dbkey).tx.clone())
            },                 
            KeyTypes::Empty => Err(RediSQLError::empty_key()),
            _ => Err(RediSQLError::no_redisql_key()),
        }
    }

while this other is the refactored version that does not work.

    fn get_dbkey(&self) -> Result<std::mem::ManuallyDrop<DBKey>, RediSQLError> {
        match self.key_type() {
            KeyTypes::RediSQL => unsafe {   
                let dbkey = {
                    rm::ffi::RedisModule_ModuleTypeGetValue.unwrap()(
                        self.key,                       
                    ) as *mut DBKey                 
                };
    
                let dbkey = std::ptr::read(dbkey);
                let dbkey = std::mem::ManuallyDrop::new(dbkey);
                Ok(dbkey)      
            },
            KeyTypes::Empty => Err(RediSQLError::empty_key()), 
            _ => Err(RediSQLError::no_redisql_key()),
        }
      
    } 
    pub fn get_channel(        
        &self,
    ) -> Result<ManuallyDrop<Sender<Command>>, RediSQLError> where {
        let dbkey = self.get_dbkey()?;  
        Ok(dbkey.tx.clone())   
    }

Where DBKey is defined in both cases as:

pub struct DBKey {
    pub tx: ManuallyDrop<Sender<Command>>,
    pub loop_data: Loop,
    pub connections: HashMap<String, Sender<Command>>,
    pub context: Option<Context>,
}

If somebody got any idea...

[Yandros]

@siscia

let ptr = unsafe { ... FFI call ...} as *mut DBKey; // get raw pointer
let dbkey = std::ptr::read(ptr); // read the raw pointer
let dbkey = ManuallyDrop::new(dbkey) // avoid to drop the structure

And I obtain the sender like this:

... continuing from above...
let dbkey : ManuallyDrop<DBKey> = { ... function above ... }
Ok(dbkey.tx.clone()) // clone the Sender wrapped in the ManuallyDrop above

You do not need the read-into-ManuallyDrop pattern here, just do

let ptr = unsafe { ... FFI call ... } as *const DBKey;
let db_key: &DBKey = unsafe { // shared ref suffices
    // guard let Some(db_key) = ptr.as_ref() else {
    if let Some(it) = ptr.as_ref() { it } else {
        panic!("Got NULL ptr from FFI at ..."); // or `return Err(...);`
    }
};
Ok(db_key.tx.clone())

Also, there is an issue with ManuallyDrop<Sender>: it may indeed be crucial for some safety somewhere (I haven't spotted where by skimming at the code), but if you have ManuallyDrops without any ManuallyDrop::into_inner() or unsafe { ManuallyDrop::drop(...) } calls, then for sure there are leaks happening.

---

Note: neither spotted "issue" should be causing the Senders to be dropped "too much", quite the opposite! So my suggested changes, although nice to do, will not fix your "dropped senders issue"

[siscia]

Sorry for the late reply.

Indeed they should be leaked, not be dropped.

Unfortunately the code is more complex of what I can show here (not private, but simply too big).

If I figure out the issue I will report back.

[siscia]

I made some advancement on this. The Sender was panic on clone (???), but it was panicking inside a orphan thread that I had no visibility into.

I am gathering more issue with the intend to open a new discussion thread or to report back the problem.

[siscia]

Follow up here: Sender panic on `Clone`, in some particolar condition