MailKitSimplified icon indicating copy to clipboard operation
MailKitSimplified copied to clipboard

Monitor multiple boxes threads issue

Open Sylar-A opened this issue 1 year ago • 30 comments

Hi, @danzuep!

At first, thanks a lot for the monitoring multiple boxes feature! At second, there is some issue with it. I have registered 7 FolderMonitors in appsettings and using IMailFolderMonitorFactory.MonitorAllMailboxesAsync method to monitor them. So, for example (and not only this example), when i get 2 mail in 1 mailbox same time, it's thows exceptions:

MailKitSimplified.Receiver.Services.MailFolderMonitor|IMAP client is being accessed by multiple threads. System.InvalidOperationException: The ImapClient is currently busy processing a command in another thread. Lock the SyncRoot property to properly synchronize your threads.
   at MailKit.Net.Imap.ImapEngine.PopNextCommand()
   at MailKit.Net.Imap.ImapEngine.IterateAsync()
   at MailKit.Net.Imap.ImapEngine.RunAsync(ImapCommand ic)
   at MailKit.Net.Imap.ImapClient.NoOpAsync(CancellationToken cancellationToken)
   at MailKitSimplified.Receiver.Services.MailFolderMonitor.ProcessMessagesArrivedAsync(Boolean firstConnection, CancellationToken cancellationToken)
   at MailKitSimplified.Receiver.Services.MailFolderMonitor.WaitForNewMessagesAsync(CancellationToken cancellationToken)
MailKitSimplified.Receiver.Services.MailFolderMonitor|Error occurred processing arrival queue item #MailKit.MessageSummary. System.InvalidOperationException: The ImapClient is currently busy processing a command in another thread. Lock the SyncRoot property to properly synchronize your threads.
   at MailKit.Net.Imap.ImapEngine.PopNextCommand()
   at MailKit.Net.Imap.ImapEngine.IterateAsync()
   at MailKit.Net.Imap.ImapEngine.RunAsync(ImapCommand ic)
   at MailKit.Net.Imap.ImapFolder.GetBodyPartAsync(UniqueId uid, String partSpecifier, CancellationToken cancellationToken, ITransferProgress progress)
   at MailKitSimplified.Receiver.Extensions.MessageSummaryExtensions.GetBodyTextAsync(IMessageSummary messageSummary, CancellationToken cancellationToken)
   at MailKitSimplified.Receiver.Services.MailFolderMonitor.ProcessArrivalQueueAsync(Func`2 messageArrivalMethod, CancellationToken cancellationToken)

At third, there is example with worker which is Singleton and have injected from constructor dependency on IImapReceiver, which is Transient. So, it won't be transient, WorkerService will control it's lifetime, and it would be Singleton. There is recomendation from Microcoft to do it rigth way.

Exceptions throws every day several times😥 Please, help, Daniel🙏

Sylar-A avatar May 18 '24 13:05 Sylar-A

Hi @Sylar-A, glad you found the FolderMonitor feature as I'm not sure I'd documented it fully 😊. I ran it in my test environment but the project I initially wanted to use it for has been and gone, so I haven't actually used it in a real-world setting yet. It sounds like you've figured out the fix for this issue though, so well done! Please make a fork of the project, test your proposed changes on your machine, then make a pull request to merge your changes into the main branch. Then your name will appear on the main page as a contributor 🙂

danzuep avatar May 19 '24 03:05 danzuep

Hi, @danzuep! I have found a fix for another issue in sample code, but it's not fix my issue with multiple threads😞

Sylar-A avatar May 19 '24 03:05 Sylar-A

As a short-term workaround you could try using the ImapReceiverFactory instead. If that works then using the copy function could be the solution.

danzuep avatar May 19 '24 15:05 danzuep

Hi, @danzuep!

If i understand you correct, it must be a BackgroundService with while loop and with some timeout? For example:

    private async Task DoWork(CancellationToken cancellationToken)
    {
        while (!cancellationToken.IsCancellationRequested)
        {
            await Task.Delay(TimeSpan.FromMinutes(1), cancellationToken);

            foreach (var receiver in _imapReceiverFactory.GetAllImapReceivers())
            {
                var messageSummaries = await receiver.ReadMail.GetMessageSummariesAsync(cancellationToken);

                foreach (var messageSummary in messageSummaries)
                {
                    // Do something...
                }
            }
        }
    }

But i didn't realize how to use the copy method as you suggested.

Sylar-A avatar May 20 '24 05:05 Sylar-A

The method copy is used to ensure each receiver has a unique IMAP client, but it's used inside the receiver factory and the idle client, so that should be enough. You could try adding Task.WhenAll with mailbox monitors to your code above, or try modifying the monitor factory directly to find the source of the error. Enable debug logging to see further into the stack, breakpoints are good too but harder to follow with all the multi-threading. I won't be able to do this myself for another week or so, much better for both of us if you can find the issue before then 🙂

danzuep avatar May 20 '24 10:05 danzuep

Oh, ok, Daniel, thanks for parting words, i'll try to hard debug it😅

Sylar-A avatar May 20 '24 10:05 Sylar-A

Looking back at the error message, it says the "IMAP client is being accessed by multiple threads." The IMAP client isn't being passed through when the MailFolderMonitor is constructed though. Then I saw the second part that says "GetBodyTextAsync(IMessageSummary)". The short term fix then is not to use messageSummary.Folder. Hopefully that helps too with a more targeted search 🙂

danzuep avatar May 22 '24 00:05 danzuep

Hi, @danzuep!

I have found another issue with IImapReceiverFactory. When it's creates new receivers and all of them have same ImapHost, it's takes only one mail (in my case one same mail seven times instead of 7 different mails). When I debugging it, the problem was in IMemoryCache extension method GetOrCreate (from Microsoft). Because of all mails have same ImapHost, it wasn't creates new item and tooks the same item 7 times by the key. It was here.

Sylar-A avatar May 24 '24 05:05 Sylar-A

Wow, good find! _memoryCache can be removed, it doesn't add much in terms of functionality or speed. Are you happy to add it to a pull request? If so, please submit each fix as a separate pull request if possible, but all good if not.

danzuep avatar May 24 '24 05:05 danzuep

Ok, Daniel. I could do it later.

Sylar-A avatar May 24 '24 05:05 Sylar-A

Hey @Sylar-A, is the issue you found fixed?

You also mentioned changing to scoped services, but I don't see those in the fork on your profile. I've done that for the Sender, but I guess I never got around to it for the Receiver so I should get on to it.

danzuep avatar Jun 09 '24 01:06 danzuep

Hi, @danzuep! Saddenly no🥲 I cant reproduce it in tests. I've created test email address and sent many messages same time and it worked perfectly. But it still crashing in production every day( Maybe the reason is that there is only one test email address and i must test it with many. I dont know...

Scoped services would be great! But you have to remember that if you inject scoped (or transient) service from background servise (singleton) constructor, background service will create it only once.

Sylar-A avatar Jun 09 '24 16:06 Sylar-A

The answer to this issue you've found is to not use use the messageSummary that comes from the MailFolderMonitor or the messageSummary.Folder. I've added a MailFolderCache function you can use from the ImapReceiver, so use the UniqueId and the GetMailFolderAsync() function instead of using the MessageSummary directly. One possible solution would be for me to just return the UniqueId instead of returning the MessageSummary.

danzuep avatar Jun 10 '24 02:06 danzuep

Here's a sample using the latest pre-release:

        using var scope = _serviceScopeFactory.CreateScope();
        var mailFolderClient = scope.ServiceProvider.GetRequiredService<IMailFolderClient>();
        var mailFolderMonitorFactory = scope.ServiceProvider.GetRequiredService<IMailFolderMonitorFactory>();
        async Task UniqueIdArrivedAsync(IMessageSummary messageSummary) =>
            await mailFolderClient.MoveToAsync(messageSummary, destinationFolderFullName, cancellationToken);
        await mailFolderMonitorFactory.MonitorAllMailboxesAsync(UniqueIdArrivedAsync, cancellationToken);

danzuep avatar Jun 10 '24 07:06 danzuep

Please test the changes and let me know how it goes.

danzuep avatar Jun 10 '24 07:06 danzuep

Hi, @danzuep! I cant find the latest pre-release as you mentioned in nuget. There is only previous version. I've opened it with dotPeek and there aren't your last changes. Maybe it's GitHub issue, because there is only one rc tag on 2.10.0 too. image

Sylar-A avatar Jun 10 '24 15:06 Sylar-A

Ah. it seems that GitVersion_NuGetVersion didn't update, even though the semantic version is correct. Weird, that used to work. I re-ran it with a higher version and it published this time.

danzuep avatar Jun 11 '24 00:06 danzuep

Hi, @danzuep! I've found some issues with new code🥲

  1. When i try to call await mailFolderClient.AddFlagsAsync([messageSummary.UniqueId], MessageFlags.Seen); it throws an exception:
MailKit.Security.AuthenticationException: invalid command
   at MailKit.Net.Imap.ImapClient.AuthenticateAsync(Encoding encoding, ICredentials credentials, CancellationToken cancellationToken)
   at MailKitSimplified.Receiver.Services.ImapReceiver.AuthenticateImapClientAsync(CancellationToken cancellationToken)
   at MailKitSimplified.Receiver.Services.ImapReceiver.ConnectAuthenticatedImapClientAsync(CancellationToken cancellationToken)
   at MailKitSimplified.Receiver.Services.ImapReceiver.ConnectMailFolderAsync(CancellationToken cancellationToken)
   at MailKitSimplified.Receiver.Services.MailFolderClient.ConnectMailFolderAsync(IMailFolder mailFolder, Boolean enableWrite, CancellationToken cancellationToken)
   at MailKitSimplified.Receiver.Services.MailFolderClient.ConnectAsync(Boolean enableWrite, CancellationToken cancellationToken)
   at MailKitSimplified.Receiver.Services.MailFolderClient.AddFlagsAsync(IEnumerable`1 uniqueIds, MessageFlags messageFlags, Boolean silent, CancellationToken cancellationToken)

Somewhere here (deeper in MailKit): image

  1. When i try to call await mailFolderClient.MoveToAsync(messageSummary, _archiveFolderName); it throws an NullReferenceException with _mailFolderCache because of mailFolder is null:
System.NullReferenceException: Object reference not set to an instance of an object.
   at MailKitSimplified.Receiver.Services.MailFolderCache.CacheMailFolder(IMailFolder mailFolder)
   at MailKitSimplified.Receiver.Services.MailFolderCache.GetMailFolderAsync(IImapReceiver imapReceiver, String mailFolderFullName, CancellationToken cancellationToken)

image

Sylar-A avatar Jun 11 '24 17:06 Sylar-A

It sounds like the mail folder name is not found. What do the protocol logs say? Do the folder names match?

danzuep avatar Jun 12 '24 14:06 danzuep

ProtocolLog is empty. I didn't change the folder name. It's same name that i pass to await messageSummary.Folder.GetSubfolderAsync(_archiveFolderName);.

Sylar-A avatar Jun 12 '24 15:06 Sylar-A

Some of the issues you mentioned are fixed in the latest pre-release, try updating. The main thing I haven't tried is creating folders, but the other ones should work now. The root of the issue was mistakenly using the IMAP client directly without connecting or authenticating it first.

danzuep avatar Jun 19 '24 17:06 danzuep

Hi, Daniel! Thank you for fixes! But I still can't see new rc version in nuget packages🥲 Maybe you'll release a new version after all fixes?

Sylar-A avatar Jun 19 '24 18:06 Sylar-A

Looks like it was there when you commented, but just in case I've made a new one. Please do a Pull Request if you notice any issues.

danzuep avatar Jun 20 '24 01:06 danzuep

Hi, @danzuep! Second point of comment was fixed, thanks! But first is still throws and now on both mailFolderClient.AddFlagsAsync() and mailFolderClient.MoveToAsync() methods( Maybe the problem is in empty ImapCredentials here? It's filled in appsettings.json file. image

Sylar-A avatar Jun 21 '24 15:06 Sylar-A

It seems it's a problem with your authentication settings, but the issue isn't clear from the error message (MailKit.Security.AuthenticationException: invalid command). What's the ProtocolLog saying?

danzuep avatar Jun 21 '24 15:06 danzuep

It says

Connected to imaps://imap.mail.ru:993/
S: * OK IMAP4 ready
C: V00000000 CAPABILITY
S: * CAPABILITY IMAP4rev1 ID XLIST UIDPLUS UNSELECT MOVE LIST-STATUS SASL-IR AUTH=PLAIN AUTH=XOAUTH2
S: V00000000 OK CAPABILITY completed
C: V00000001 AUTHENTICATE PLAIN ********
S: V00000001 NO [AUTHENTICATIONFAILED] NEOBHODIM parol prilozheniya https://help.mail.ru/mail/security/protection/external / Application password is REQUIRED
C: V00000002 LOGIN "" ""
S: V00000002 BAD invalid command

Sylar-A avatar Jun 21 '24 16:06 Sylar-A

"Application password is REQUIRED" (Gmail has the same requirement). The capability line includes AUTH=XOAUTH2, if you you just want to get it working, you can use imapReceiver.RemoveAuthenticationMechanism("XOAUTH2");.

Authenticating via a SASL mechanism may be a multi-step process. see href="http://www.mimekit.net/docs/html/T_MailKit_Security_SaslMechanism.htm" seealso href="http://www.mimekit.net/docs/html/T_MailKit_Security_SaslMechanismOAuth2.htm"

danzuep avatar Jun 22 '24 02:06 danzuep

Hi, @danzuep! The error is not in XOAUTH2 mechanism, it's in PLAIN. In this image you can see that credentials are not empty image In this image it's empty (when i use mailFolderClient methods AddFlagsAsync and MoveToAsync) image log:

Connected to imaps://imap.mail.ru:993/
S: * OK IMAP4 ready
C: V00000000 CAPABILITY
S: * CAPABILITY IMAP4rev1 ID XLIST UIDPLUS UNSELECT MOVE LIST-STATUS SASL-IR AUTH=PLAIN AUTH=XOAUTH2
S: V00000000 OK CAPABILITY completed

Sylar-A avatar Jun 23 '24 14:06 Sylar-A

Hi @danzuep! Any decision with this bug? I suppose the problem is in resolving configs in new instances of services (after register them as scope/transient). I can't use your new code because of it. Please, help🙏

Sylar-A avatar Jul 30 '24 11:07 Sylar-A

I haven't been able to replicate the issue you're having sorry. Have you tried connecting to GMail or one of the other big providers? Try downloading the source code and using breakpoints to pinpoint the issue.

danzuep avatar Jul 30 '24 12:07 danzuep