Dont trust me Dont trust me -4 years ago 138
C# Question

ASP.Net C# Active Directory: Inherited an application with thread.sleep() after each DirectoryEntry.MoveTo() calls

So I'm working on this application that was developed by other people that are not involved with the project anymore. As stated in the title there are a few thread.sleep() calls that have me scratching my head. I have no experience working with AD.

The worst offender is the following one with a 50 second sleep in a class that inherits

Principle
.

public class Computer:ComputerPrinciple
{
...
public void MoveToOu(string ou)
{
var userObject = (DirectoryEntry)GetUnderlyingObject();
var newParentOu = new DirectoryEntry("LDAP://" + ou);

userObject.MoveTo(newParentOu);

Thread.Sleep(50000);
userObject.Close();
newParentOu.Close();
}
...
}


There is nothing that explains the reason but I suspect it was an issue with the page refreshing before the changes were fully committed (?). One thing I'm pretty sure of is that
Thread.Sleep(50000)
is not the correct solution to whatever problem they we're having.

I'd like to hear from people who have experience with this if there is indeed some sort of issue that this would "fix" and what would be the incorporate solution.

Also I'm thinking I should use the "using" directive but I'm wondering about disposing the
UnderlyingObject
or even just closing it because this code is called from something like this where computer is the principle in witch the move code is

computer.UpdateComputerDescription(collection);
computer.UpdateComputerOu(collection); <--- offending MoveToOu method is called in here
computer.UpdateComputerIsActivedProperty(collection);
computer.Save();


Would closing or disposing of the
underlyingObject
have a side effect on the methods after that call?

Any input would be welcome.

Answer Source

This use of Thread.Sleep is likely a workaround for not calling CommitChanges.

From documentation

If UsePropertyCache is true, call the CommitChanges method on the new object to make the move permanent

It looks like they were waiting for the cache to flush using an empirical delay.

Second, wrapping any IDisposable you create inside MoveToOu in using will not break anything. It is the correct way to go. Objects used, but not created, inside MoveToOu scope should not be disposed here, but rather be left to the container class dispose mechanism.

Recommended from our users: Dynamic Network Monitoring from WhatsUp Gold from IPSwitch. Free Download