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

MimeMessage.WriteTo is not thread-safe even in different instances of MimeMessage #138

Closed
ymassad opened this issue May 10, 2015 · 5 comments
Labels
bug Something isn't working

Comments

@ymassad
Copy link

ymassad commented May 10, 2015

If we have multiple MimeMessage instances, say 1000 instance. And I try to call WriteTo in parallel for these messages I will get some corrupted messages.

Here is a sample code:

        string filename = @"C:\simple.eml";

        byte[] data = File.ReadAllBytes(filename);

        ConcurrentBag<MimeMessage> messages = new ConcurrentBag<MimeMessage>();

        for(int i = 0 ; i < 1000 ; i++)
        {
            messages.Add(MimeMessage.Load(new MemoryStream(data)));
        }

        int length = data.Length;

        Parallel.For(0, 1000, (int i) =>
        {
            //lock(data)
            { 
                MemoryStream ms = new MemoryStream();

                var msg = messages.Take(1).First();

                msg.WriteTo(ms);

                var message_data = ms.ToArray();

                if (message_data.Length != length)
                {
                    //There is an error
                    int ggg = 0;
                }
            }
        });

The corrupted messages have either missing or extra mime headers.

If I uncomment the lock statement, everything works fine.

Please note that in the example, each thread/task is working with a different instance of MimeMessage

The simple.eml file is a very simple email file with no attachments. I have tried different files, and the problem happened for all of them.

It seems to me that there might be some shared state for different instances of MimeMessage

@ymassad
Copy link
Author

ymassad commented May 10, 2015

Instead of

                var msg = messages.Take(1).First();

It makes more sense to use

                MimeMessage msg;

                messages.TryTake(out msg);

This, however, does not fix the problem.

@jstedfast
Copy link
Owner

This will fix it:

var options = FormatOptions.Default.Clone ();
var msg = messages.Take(1).First();

msg.WriteTo(options, ms);

@ymassad
Copy link
Author

ymassad commented May 10, 2015

Thanks.

I tried passing FormatOptions.Default.Clone() to the WriteTo method and the problem is solved.

I see that you have made some changes to the code base. I did not get this new code, but the issue is solved. Do you think there is a chance I will have any problems using the code base I currently have and just passing FormatOptions.Default.Clone() to the WriteTo method?

@jstedfast
Copy link
Owner

Yes, you should be fine.

@ymassad
Copy link
Author

ymassad commented May 10, 2015

Thanks

@jstedfast jstedfast added the bug Something isn't working label May 12, 2015
jstedfast added a commit that referenced this issue May 31, 2015
Added an internal HeaderList.Suppress property to hold the state instead.

This is a far better fix for issue #138
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