Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Monitor multiple boxes threads issue #63

Open
Sylar-A opened this issue May 18, 2024 · 30 comments
Open

Monitor multiple boxes threads issue #63

Sylar-A opened this issue May 18, 2024 · 30 comments
Assignees
Labels
bug Something isn't working

Comments

@Sylar-A
Copy link
Contributor

Sylar-A commented May 18, 2024

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🙏

@danzuep
Copy link
Owner

danzuep commented May 19, 2024

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 🙂

@Sylar-A
Copy link
Contributor Author

Sylar-A commented May 19, 2024

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

@danzuep
Copy link
Owner

danzuep commented May 19, 2024

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

@Sylar-A
Copy link
Contributor Author

Sylar-A commented May 20, 2024

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.

@danzuep
Copy link
Owner

danzuep commented May 20, 2024

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 🙂

@Sylar-A
Copy link
Contributor Author

Sylar-A commented May 20, 2024

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

@danzuep danzuep added the bug Something isn't working label May 20, 2024
@danzuep
Copy link
Owner

danzuep commented May 22, 2024

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 🙂

@Sylar-A
Copy link
Contributor Author

Sylar-A commented May 24, 2024

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.

@danzuep
Copy link
Owner

danzuep commented May 24, 2024

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.

@Sylar-A
Copy link
Contributor Author

Sylar-A commented May 24, 2024

Ok, Daniel. I could do it later.

@danzuep
Copy link
Owner

danzuep commented Jun 9, 2024

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.

@Sylar-A
Copy link
Contributor Author

Sylar-A commented Jun 9, 2024

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.

@danzuep
Copy link
Owner

danzuep commented Jun 10, 2024

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
Copy link
Owner

danzuep commented Jun 10, 2024

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
Copy link
Owner

danzuep commented Jun 10, 2024

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

@Sylar-A
Copy link
Contributor Author

Sylar-A commented Jun 10, 2024

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

@danzuep
Copy link
Owner

danzuep commented Jun 11, 2024

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.

@Sylar-A
Copy link
Contributor Author

Sylar-A commented Jun 11, 2024

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

@danzuep
Copy link
Owner

danzuep commented Jun 12, 2024

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

@Sylar-A
Copy link
Contributor Author

Sylar-A commented Jun 12, 2024

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

danzuep added a commit that referenced this issue Jun 19, 2024
@danzuep
Copy link
Owner

danzuep commented Jun 19, 2024

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.

@Sylar-A
Copy link
Contributor Author

Sylar-A commented Jun 19, 2024

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?

@danzuep
Copy link
Owner

danzuep commented Jun 20, 2024

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.

@Sylar-A
Copy link
Contributor Author

Sylar-A commented Jun 21, 2024

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

@danzuep
Copy link
Owner

danzuep commented Jun 21, 2024

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?

@Sylar-A
Copy link
Contributor Author

Sylar-A commented Jun 21, 2024

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

@danzuep
Copy link
Owner

danzuep commented Jun 22, 2024

"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 added a commit that referenced this issue Jun 22, 2024
@Sylar-A
Copy link
Contributor Author

Sylar-A commented Jun 23, 2024

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
Copy link
Contributor Author

Sylar-A commented Jul 30, 2024

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🙏

@danzuep
Copy link
Owner

danzuep commented Jul 30, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants