madhatter22 / LinqToLdap

C# LINQ provider built on top of System.DirectoryServices.Protocols for querying and updating LDAP servers.
MIT License
45 stars 23 forks source link

Multiple requests at the same time #10

Closed veezed closed 4 years ago

veezed commented 4 years ago

image

This code can't run. If I use only 1 thread all is fine, but at least with 2 it breaks. Please explain maybe I am doing something wrong?

at System.DirectoryServices.Protocols.LdapConnection.BindHelper(NetworkCredential newCredential, Boolean needSetCredential) at System.DirectoryServices.Protocols.LdapConnection.SendRequestHelper(DirectoryRequest request, Int32& messageID) at System.DirectoryServices.Protocols.LdapConnection.SendRequest(DirectoryRequest request, TimeSpan requestTimeout) at System.DirectoryServices.Protocols.LdapConnection.SendRequest(DirectoryRequest request) at LinqToLdap.QueryCommands.StandardQueryCommand.HandleStandardRequest(DirectoryConnection connection, ILinqToLdapLogger log, Int32 maxSize, Boolean pagingEnabled) at LinqToLdap.QueryCommands.StandardQueryCommand.Execute(DirectoryConnection connection, SearchScope scope, Int32 maxPageSize, Boolean pagingEnabled, ILinqToLdapLogger log, String namingContext) at LinqToLdap.DirectoryQueryProvider.Execute(Expression expression) at LinqToLdap.QueryProvider.Execute[TResult](Expression expression) at LinqToLdap.QueryableExtensions.ToList[TSource](IQueryable1 source) at LDAP.Program.<>c__DisplayClass2_0.<MainAsync>b__0(Int32 x) in C:\Users\vzaramba\Desktop\LDAP\LDAP\Program.cs:line 48 at System.Threading.Tasks.Parallel.<>c__DisplayClass17_01.b1() at System.Threading.Tasks.Task.InnerInvoke() at System.Threading.Tasks.Task.InnerInvokeWithArg(Task childTask) at System.Threading.Tasks.Task.<>cDisplayClass176_0.b__0(Object )

madhatter22 commented 4 years ago

Hi, thanks for reporting this. Are you using 4.0 or 4.1 preview? Also which version of .Net?

veezed commented 4 years ago

Hi, I tried both versions, 4.0 and 4.1. Version of .NET is 4.6. Will try other .NET versions, but don't thing that it will change anything.

madhatter22 commented 4 years ago

It will probably error on all. I just wanted to make sure I had the correct test coverage. Thanks again.

madhatter22 commented 4 years ago

I see the problem. It's been a while since I've looked at the pooled factory code. When a DirectoryContext is created it gets a connection from the pool and returns it to the pool when it is disposed. The underlying LdapConnection is not thread safe. You should change your CreateDirectoryContext method to CreateLdapConfiguration. And then your Parallel.For should change to this:

Parallel.For(0, 10, new ParallelOptions { MaxDegreeOfParallelism = 4 }, x => { using (var context = new DirectoryContext(config) / or config.CreateContext() /) { var list = context.Query\<User>().Where(u => u.LastName== "the name").ToList(); } });

veezed commented 4 years ago

My code is just as an example. I made it when I tried to debug where is the problem, why we get those type of errors. Real world situation is a bit different. We use CQRS pattern in a multi threaded environment. We create context only once when we initialize application.

You are saying that each thread should have its own directory context?

On Sun, Nov 3, 2019 at 10:19 PM Alan Hatter notifications@github.com wrote:

I see the problem. It's been a while since I've looked at the pooled factory code. When a DirectoryContext is created it gets a connection from the pool and returns it to the pool when it is disposed. The underlying LdapConnection is not thread safe. You should change your CreateDirectoryContext method to CreateLdapConfiguration. And then your Parallel.For should change to this:

Parallel.For(0, 10, new ParallelOptions { MaxDegreeOfParallelism = 4 }, x => { using (var context = new DirectoryContext(config) / or config.CreateContext() /) { var list = context.Query().Where(u => u.LastName== "the name").ToList(); } });

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/madhatter22/LinqToLdap/issues/10?email_source=notifications&email_token=ABCD4FR76STDXZEDUTZXNKTQR4P5VA5CNFSM4JGVHZY2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC53ZUA#issuecomment-549174480, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABCD4FXRAOJI57LVGS2SKALQR4P5VANCNFSM4JGVHZYQ .

-- Valdas Zaramba

madhatter22 commented 4 years ago

Correct. I can't find the documentation, but Microsoft did not make the LdapConnection object thread-safe so you need a connection per thread. I wrote a blog post years ago (.Net 4.0 era) on using connection pooling in a multi-threaded environment: http://hattercoding.blogspot.com/2011/04/linq-to-ldap-connection-pooling.html.

Also as an FYI I am also using this library in a CQRS service running on top of Web API (and .Net Core using 4.1-preview for testing).