[Mono-dev] Ldap and Threading

[Mike Glenn]

First let me say again that these are JD Conley's changes ( He emailed the list
a few days ago on this thread, if you need his email address ) so my
explanations are mostly just guesses as to my understanding of what's happening
in the code, which I'll be the first to admit is not 100%. Nothing I say should
be seen as pointing a finger at JD. What I say and why he made the change could
be completely different. So with that in mind:

> I looked into the BoiledDown patch, here are my comments:
> 
> 1. '\n' -> Environment.NewLine changes. Can you provide some
> explanations / test case(s) for this?

Those changes are in the LDAPException class and section in question deals with
returning a text message concerning the Exception chain that occurred.
Environment.NewLine here would accomplish two things. 

1. make the returned messages line return correctly regardless of platform. I
only work on the x86 platform with windows and linux, where \n works fine, but
I'm sure that will not work on all platforms that mono is or could be ported to.


2. By using NewLine you avoid extra object creation that occurs when you define
chars and strings like that and easy the GC burden by the fact that it doesn't
have to clean up the Environment.NewLine like it does with every '\n' that goes
out of scope.

Neither of these have anything to do with the issue at hand. And I apologize as
they should have been dealt with in a separate patch I guess. But in my defense
I was merging what amounted to three different trees (Novell's, JD's, and mono's
changes) :-)

> 2. The rest of the patch deals with correct catching and 
> passing of the
> exceptions thrown in the "working" thread to the "user" thread. This
> definitely solves the "hang" issue, but does nothing with the cause of
> those exceptions. So the route bug(s) remain (correct me if 
> I'm wrong). 

I believe you are correct to the limits of my understanding, but I do see some
changes in the MessageAgent.cs and MessageVector.cs classes that are locking
related. Mostly around using the collections SyncRoot instead of the collection
object itself for the lock. My assumption when I reviewed the code was that
these changes provided a more reliable lock on the collection and as such
prevented the race when accessing the returned messages. 

Maybe JD could jump in here and provide some explanation of his reasoning for
these locking changes and what he was addressing by making them. The rest of the
changes we seem to agree on what their reason and results are.

Mike Glenn