How To Make This Async [UPDATED]

Kevin Marois

public async Task<List<ActiveDirectoryUser>> ImportUsersAsync(string basePath)
{
return await Task.Run(() =>
{
List<ActiveDirectoryUser> users = new List<ActiveDirectoryUser>();

    var rootEntry = new DirectoryEntry(basePath);
    var searcher = new DirectorySearcher(rootEntry);
    SearchResultCollection results = searcher.FindAll();

    if (results != null)
    {
        foreach (SearchResult result in results)
        {
            ActiveDirectoryUser user = new ActiveDirectoryUser();
            if (result.Properties.Contains("displayname"))
            {
                user.DisplayName = (string)result.Properties\["displayname"\]\[0\];
                user.LoginName = (string)result.Properties\["samaccountname"\]\[0\];

                foreach (var prop in result.Properties)
                {
                    DictionaryEntry de = (DictionaryEntry)prop;
                    ResultPropertyValueCollection rpvc = (ResultPropertyValueCollection)de.Value;
                    user.Properties.Add(de.Key.ToString(), rpvc\[0\].ToString());
                }

                users.Add(user);
            }
        }
    }

    return users;

});

}

now, when i run it:

ActiveDirectoryHelper adHelper = new ActiveDirectoryHelper();

var users = adHelper.ImportUsersAsync(basePath); // This is async

var user = adHelper.GetUserInfo(serverName, userName);

Console.WriteLine("Done");

How can I know when the async method finished?

If it's not broken, fix it until it is. Everything makes sense in someone's mind. Ya can't fix stupid.

Dave Kreskowiak

You don't. The way you set it up, it's "fire and forget". You've got the await in the wrong place. That should go on the callers side, not the function ...Async side. The Async method should return a Task. That lets the caller await the Task. Something like this:

private Task> GetUsersAsync()
{
return Task.Run(() =>
{
List userList = new List() {
"User1", "User2", "User3", "User4", "User5"
};

    Thread.Sleep(5000);

    return userList;
});

}

Then you can call it like this (very simple Windows Forms example):

private async void button1_Click(object sender, EventArgs e)
{
// The await just says "Put a bookmark here and return from the method until the Task returns".
// In this simple Windows Forms example, the code will go back to processing the message pump,
// keeping the UI alive.
var users = await GetUsersAsync();

// When the Task returns, the code will resume here and show the list that was returned.
foreach (string user in users)
{
    listBox1.Items.Add(user);
}

}

Is this production quality code? NO! It's meant to be a simple example demonstrating Task/Async/Await. It doesn't handle exceptions and doesn't make sure the Click handler doesn't get called again while the first Task is still running.

System.ItDidntWorkException: Something didn't work as expected. C# - How to debug code[^]. Seriously, go read these articles.
Dave Kreskowiak

User 13485779

var users = adHelper.ImportUsersAsync(basePath); // This is async

change in

var users = adHelper.ImportUsersAsync(basePath).Result; // This is async

Kevin Marois

Thank you!

If it's not broken, fix it until it is. Everything makes sense in someone's mind. Ya can't fix stupid.

Dave Kreskowiak

That's not going to fix the problem at all and you're really not even changing the code. The .Result is implied in the original post.

System.ItDidntWorkException: Something didn't work as expected. C# - How to debug code[^]. Seriously, go read these articles.
Dave Kreskowiak

Alex Schunk

Oh no... That Result there is making a sync method out of the async method. And Result shouldn't be used like that... You are asking for deadlocks that way.

Alex Schunk

No it is not implied... He is just assigning a non started Task to user. As long as he doesnt await user() or user.Result... Then nothing ever happens with it.