Update documentation to mention that magic_open isn't thread safe (new Magic()) about mime HOT 12 CLOSED

How to reproduce it?

I'm trying to create threads without pool like this:

using System;
using System.IO;
using System.Threading;

using HeyRed.Mime;

namespace TestApp
{
    class Program
    {
        static void Main(string[] args)
        {
            var files = Directory.GetFiles("/path/to/files");

            for (int i = 0; i < 500; i++)
            {
                Thread t = new Thread(() => 
                {
                    string filePath = files[i];

                    var magic = new Magic(MagicOpenFlags.MAGIC_MIME_TYPE);

                    string mimeType = magic.Read(filePath);

                    Console.WriteLine($"File: {filePath}: {mimeType}");
                });

                t.Start();
            }
        }
    }
}

But it works as expected.
Btw if we need lock inside constructor, then we need it around all methods that works with pointer.

from mime.

It happened to me in Parallel.ForEach on a big (1000s of items) array and creating a Magic instance inside the body lambda on quad-core machine.
It would randomly crash with Heap corruption when the instance got used and randomly during creation also.

from mime.

@rwasef1830 Host is Linux?

from mime.

No, I was running my app on Windows 10. Once I locked around Magic instantiation, I could use the multiple instances simultaneously without problem.

from mime.

Here's the commit where I fixed it: rwasef1830/grepripper@6d62c3c

from mime.

Hmm, when I'm share single instance of Magic I got same issue, but if I create new instance in parallel loop it's work as expected.
So yes, it's covered in remarks
I think we should leave it as it is, because Magic is low level abstraction and most users not needed to go deeper than MimeGuesser.
But I'm should hightlight it in readme anyway.
Thanks for report 👍

from mime.

Also, I'm see golang implementation and its uses mutex for all operations
https://github.com/rakyll/magicmime/blob/master/magicmime.go#L181
🤔

from mime.

@hey-red the problem is that magic_open called in the constructor is not safe, so even the constructor needs to be locked. You can add a lock inside around magic_open, because libmagic changes shared memory on library initialization.

People can realize that magic instances aren't thread safe, but they don't realize that even Magic creation is also not thread safe.

from mime.

@rwasef1830 Okay, seems like reasonable.
But should I use static object? Or instance based lock is fine?

from mime.

Must be private static readonly locking object. @hey-red Used ONLY for construction.
Don't put any locks in other methods. Once you make different Magic, they can all work in parallel without problem

from mime.

Done 7acf3f1

from mime.

👍

from mime.