Hi Vincent,

Thanks for reporting this. 

Clearly, this is something that needs to be addressed, but your proposed fix 
will certainly not work, since it effectively reverses 2 other fixes:

1. In Java, the files are created with no byte order mark, and some parts of 
Lucene.Net cannot read back the files when they have one.
2. Despite the using block, this approach seems to keep the file open too long, 
causing a "file already in use" error. Using WriteAllText is the only approach 
I could find to work around that issue.

I think that a more appropriate solution would be to mimic what is being done 
in Java. Namely, it doesn't use sequential file names, they are based on a 
random hash of characters.

Thanks,
Shad Storhaug (NightOwl888)


-----Original Message-----
From: Van Den Berghe, Vincent [mailto:[email protected]] 
Sent: Monday, January 2, 2017 3:38 PM
To: [email protected]
Subject: CreateTempFile is not thread-safe

Hello,

Creating AnalyzingSuggester in parallel sometimes fails with an IOException 
"The file is already used by another process".
This is due to a flaw in FileSupport.CreateTempFile, most notably this:

                    if (File.Exists(fileName))
                    {
                        attempt++;
                        continue;
                    }
                    // Create the file
                    File.WriteAllText(fileName, string.Empty, new 
UTF8Encoding(false) /* No BOM */);


If 2 or more threads execute File.Exists very close one after another, and the 
file isn't found, the first call to File.WriteAllText will succeed and the 
other ones will succeed as well since an existing file is simply overwritten.
Dependent code will attempt to use the file as if it were unique, which will 
then trigger the exception mentioned above. Yes, it happens!
Consider replacing the File.WriteAllText call (which just creates an empty 
file) with:

                           using (new FileStream(fileName, FileMode.CreateNew))
                           {
                                  // do nothing
                           }

... which will fail if the file doesn't exist, and the exception will trigger 
another attempt.  This will work better as intended.


Happy new year,

Vincent Van Den Berghe

Reply via email to