Partial work being done twice (ThreadPool.QueueUserWorkItem)

Question

I have created a newsletter system that allows me to specify which members should receive the newsletter. I then loop through the list of members that meet the criteria and for each member, I generate a personalized message and send them the email asynchronously .

When I send out the email, I am using ThreadPool.QueueUserWorkItem.

For some reason, a subset of the members are getting the email twice. In my last batch, I was only sending out to 712 members, yet a total of 798 messages ended up being sent.

I am logging the messages that get sent out and I was able to tell that the first 86 members received the message twice. Here is the log (in the order the messages were sent)

No.  Member   Date
1.   163992   3/8/2012 12:28:13 PM
2.   163993   3/8/2012 12:28:13 PM
...
85.   164469   3/8/2012 12:28:37 PM
86.   163992   3/8/2012 12:28:44 PM
87.   163993   3/8/2012 12:28:44 PM
...
798.   167691   3/8/2012 12:32:36 PM

Each member should receive the newsletter once, however, as you can see member 163992 receives message #1 and #86; member 163993 received message #2 and #87; and so on.

The other thing to note is that there was a 7 second delay between sending message #85 and #86.

I have reviewed the code several times and ruled out pretty much all of the code as being the cause of it, except for possibly the ThreadPool.QueueUserWorkItem.

This is the first time I work with ThreadPool, so I am not that familiar with it. Is it possible to have some sort of race-condition that is causing this behavior?

=== --- Code Sample --- ===

    foreach (var recipient in recipientsToEmail)
    {
        _emailSender.SendMemberRegistrationActivationReminder(eventArgs.Newsletter, eventArgs.RecipientNotificationInfo, previewEmail: string.Empty);
    }


    public void SendMemberRegistrationActivationReminder(DomainObjects.Newsletters.Newsletter newsletter, DomainObjects.Members.MemberEmailNotificationInfo recipient, string previewEmail)
    {
//Build message here .....

//Send the message
            this.SendEmailAsync(fromAddress: _settings.WebmasterEmail,
                                toAddress: previewEmail.IsEmailFormat()
                                            ? previewEmail
                                            : recipientNotificationInfo.Email,
                                subject: emailSubject,
                                body: completeMessageBody,
                                memberId: previewEmail.IsEmailFormat()
                                            ? null  //if this is a preview message, do not mark it as being sent to this member
                                            : (int?)recipientNotificationInfo.RecipientMemberPhotoInfo.Id,
                                newsletterId: newsletter.Id,
                                newsletterTypeId: newsletter.NewsletterTypeId,
                                utmCampaign: utmCampaign,
                                languageCode: recipientNotificationInfo.LanguageCode);
        }

    private void SendEmailAsync(string fromAddress, string toAddress, string subject, MultiPartMessageBody body, int? memberId, string utmCampaign, string languageCode, int? newsletterId = null, DomainObjects.Newsletters.NewsletterTypeEnum? newsletterTypeId = null)
    {
        var urlHelper = UrlHelper();
        var viewOnlineUrlFormat = urlHelper.RouteUrl("UtilityEmailRead", new { msgid = "msgid", hash = "hash" });
        ThreadPool.QueueUserWorkItem(state => SendEmail(fromAddress, toAddress, subject, body, memberId, newsletterId, newsletterTypeId, utmCampaign, viewOnlineUrlFormat, languageCode));
    }

Answer 1

Are you sure the query you are running to get the list of members to send the email to does not have duplicates in it? Are you joining to another table? What you could do is:

List<DomainObjects.Members.MemberEmailNotificationInfo> list = GetListFromDatabase();
list = list.Distinct().ToList();

Answer 2

Having 800+ threads running on the server is not a good practice! Although you are using a ThreadPool, threads are being queued on the server and run whenever old threads return back to the pool and release the resource. This might take several minutes on the server and many situations like Race Conditions or Concurrencies might happen during that time. You could instead queue one work item, over one protected list:

lock (recipientsToEmail)
{
    ThreadPool.QueueUserWorkItem(t =>
        {
            // enumerate recipientsToEmail and send email
        });
}