inejge / ldap3

A pure-Rust LDAP library using the Tokio stack
Apache License 2.0
220 stars 38 forks source link

cannot recursively call into `Core` #34

Closed fdubois1 closed 5 years ago

fdubois1 commented 5 years ago

I'm trying to synchronise users from an LDAP server with my application. To do that, I added something like this

let user_sync_task = Interval::new_interval(config.ldap.sync_interval).for_each(move |_| {
            info!("Starting user synchronization...");

            ...SYNCHRONIZATION_CODE...
            Ok(())
        }).into_future().map_err(|e| {
            error!("Error creating user synchronization task {:?}", e);
        });

        task_executor.spawn(user_sync_task);

Of course, during the synchronisation, I open an LDAP connection. (LdapConn::new(&self.url)?;) But I get this error :

thread 'tokio-runtime-worker-0' panicked at 'cannot recursively call into Core', src/libcore/option.rs:1036:5

From what I understand, I can't open an LDAP connection in a future running in a TaskExecutor. Am I right ? How can I fix that on my side, knowing that ldap connection will create a new tokio Core and run it ?

Thank you for the help

inejge commented 5 years ago

LdapConn is a synchronous adapter which construct its own Core. An asynchronous connection must be opened with LdapConnAsync::new(). Note: this is completely untested on the newer (non-core) Tokio, and probably won't work. Your best bet is is to spin up a separate thread with an old tokio-core event loop and post LDAP requests through some kind of channel.

fdubois1 commented 5 years ago

Thank you for your answer. For now, I used a work around, I open the LDAPConn on a separate thread and it works fine. Any plan to move ldap3 on the newer Tokio ?

theredfish commented 5 years ago

Hello @fdubois1 do you have a working example? We are trying to reproduce the same thing. We are facing some deadlock issues for now - Do you use channels to send and receive information between your threads?

fdubois1 commented 5 years ago

Hello @theredfish, in our case, we don't need channels to send and receive information between thread because information is kept in a mongo database. We have a collection with all users so all threads has access to the DB and can read and write in it.

What is your case? I will try to help if I can. Good luck

theredfish commented 5 years ago

Hi @fdubois1, Thank you for your help. Our use case is to authenticate the user with its username / password before to allow him to use our application. This application is written with actix-web. That's why we are trying to use channels to communicate between the tokio thread and our own separate thread where we do the user authentication. The sender (tokio / actix) ask for the authentication and the receiver (our thread) try to authenticate the user and return the response to the sender. It's a naive approach, we're just trying to make it work - then we will search for a better solution.

fdubois1 commented 5 years ago

Hi @theredfish , Do you hit the same issue as what I got ? Why can't you authenticate your user in the tokio thread ? I had the issue because it was another executor, but if I had only one executor, it was working.

By authentication you mean to call simple_bind() ?

theredfish commented 5 years ago

@fdubois1 Indeed, we are facing the same issue where we can't use the same thread that tokio. So we're trying to create our own separate thread. What is an executor here? By authentication I mean to call simple_bind() yes. I'm preparing a clean code example if you want. It will be easier for us to debug too.

theredfish commented 5 years ago

@fdubois1 you can check the code here : https://github.com/theredfish/rust-lang-examples/tree/master/ldap3 - it's not finished yet but the idea is here : do a ldap connection through channels. Is it the right way? The shared state with sender and receiver should be deleted to avoid conflicts with multiple user connections I think.

inejge commented 5 years ago

Hey @theredfish, I see that you've got a lively conversation going 😃, but could you please redirect it to an issue in your example repo, as it's getting a bit off-topic for this one? Btw, I looked at your example, and I think I understand why you get deadlocks -- specifically, your LDAP thread locks the mutex, which you then try to obtain in fn index() and can't. On a more general note, mixing std channels and locks with futures doesn't work well, you should use tokio::sync channels if you need them.

And, @fdubois1, I do plan to port ldap3 to the newer tokio once async/await stabilizes and the churn dies out a bit.

theredfish commented 5 years ago

Hi @inejge,

Thank you for your answer, we will continue to investigate :) but we were trying to apply your advices to spin up a separate thread. So it wasn't completely off-topic. I will create an issue if we still stuck. Thanks, have a good day.

LdapConn is a synchronous adapter which construct its own Core. An asynchronous connection must be opened with LdapConnAsync::new(). Note: this is completely untested on the newer (non-core) Tokio, and probably won't work. Your best bet is is to spin up a separate thread with an old tokio-core event loop and post LDAP requests through some kind of channel.

fdubois1 commented 5 years ago

Hi @theredfish ,

I looked quickly, not sure to understand why you keep both sender and receiver in the same structure. The thread check_auth only need the receiver and the other function needs the sender only. Anyway, I don't think it explain why you have an issue, but the mutex and lock could be avoided I think. And inejge talked about deadlock, but if I understood correctly, you don't have deadlock but the error : 'cannot recursively call into Core', am I right ? I don't have time to look more but will try later.

Have a nice day