Temporary filename collision

[justinormont]

System information

• OS version/distro: Windows Server 2016

Issue

When running simultaneous sweeps (multiple processes), one process tried to open a new temp file which already exists.

Unexpected exception: The file 'C:\Users\jormont\AppData\Local\Temp\2\TLC_25553D77\0' already exists

• What did you do?
  Running an AutoML macro sweep w/ four processes on the same machine.

Perhaps our temporary path names aren't unique enough?

machinelearning/src/Microsoft.ML.Data/Model/Repository.cs

Line 507 in 211c043

entry.ExtractToFile(pathTemp);

Unsure which call is used to create the TLC_... folder, but could be this one:

machinelearning/src/Microsoft.ML.Core/Environment/TlcEnvironment.cs

Line 398 in c023727

protected override IFileHandle CreateTempFileCore(IHostEnvironment env, string suffix = null, string prefix = null)

Source code / logs

Error:

Unexpected exception: The file 'C:\Users\jormont\AppData\Local\Temp\2\TLC_25553D77\0' already exists., 'System.IO.IOException'
   at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
   at System.IO.FileStream.Init(String path, FileMode mode, FileAccess access, Int32 rights, Boolean useRights, FileShare share, Int32 bufferSize, FileOptions options, SECURITY_ATTRIBUTES secAttrs, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
   at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share)
   at System.IO.Compression.ZipFileExtensions.ExtractToFile(ZipArchiveEntry source, String destinationFileName, Boolean overwrite)
   at Microsoft.ML.Runtime.Model.RepositoryReader.OpenEntryOrNull(String dir, String name) in D:\TLC\OpenSource\src\Microsoft.ML.Data\Model\Repository.cs:line 507
   at Microsoft.ML.Runtime.Model.ModelLoadContext.LoadModelOrNull[TRes,TSig](IHostEnvironment env, TRes& result, RepositoryReader rep, String dir, Object[] extra) in D:\TLC\OpenSource\src\Microsoft.ML.Data\Model\ModelLoading.cs:line 55
   at Microsoft.ML.Runtime.Model.ModelLoadContext.LoadModel[TRes,TSig](IHostEnvironment env, TRes& result, RepositoryReader rep, String dir, Object[] extra) in D:\TLC\OpenSource\src\Microsoft.ML.Data\Model\ModelLoading.cs:line 89
   at Microsoft.ML.Runtime.Data.ApplyTransformUtils.ApplyTransformToData(IHostEnvironment env, IDataTransform transform, IDataView newSource) in D:\TLC\OpenSource\src\Microsoft.ML.Data\Utilities\ApplyTransformUtils.cs:line 47
   at Microsoft.ML.Runtime.Data.ApplyTransformUtils.ApplyAllTransformsToData(IHostEnvironment env, IDataView chain, IDataView newSource, IDataView oldSource) in D:\TLC\OpenSource\src\Microsoft.ML.Data\Utilities\ApplyTransformUtils.cs:line 101
   at Microsoft.ML.Runtime.EntryPoints.TransformModel..ctor(IHostEnvironment env, IDataView result, IDataView input) in D:\TLC\OpenSource\src\Microsoft.ML.Data\EntryPoints\TransformModel.cs:line 62
   at Microsoft.ML.Runtime.Data.TextAnalytics.TextTransform(IHostEnvironment env, Arguments input) in D:\TLC\source\TextAnalytics\TextTransform.cs:line 549

[jwood803]

@justinormont @TomFinley Tried to take a look at this, but I'm wondering if the referenced code isn't there anymore.

Did a quick search and found this:

machinelearning/src/Microsoft.ML.Core/ComponentModel/AssemblyLoadingUtils.cs

Line 106 in e0c4caa

private static string GetTempPath()

It looks like it's using a GUID. If this is the correct code, would this be considered fixed already?

[justinormont]

@jwood803, thanks for taking a look.

The referenced code moved named spaces to here:

machinelearning/src/Microsoft.ML.Core/Data/Repository.cs

Line 527 in 37ed336

entry.ExtractToFile(pathTemp);

The pathTemp is created in GetPath():

machinelearning/src/Microsoft.ML.Core/Data/Repository.cs

Lines 242 to 282 in 37ed336

	/// <summary>
	/// Constructs both the relative path to the entry and the absolute path of a corresponding
	/// temporary file. If createDir is true, makes sure the directory exists within the temp directory.
	/// </summary>
	protected void GetPath(out string pathEnt, out string pathTemp, string dir, string name, bool createDir)
	{
	_ectx.Assert(!Disposed);
	_ectx.CheckValueOrNull(dir);
	_ectx.CheckParam(dir == null || !dir.Contains(".."), nameof(dir));
	_ectx.CheckParam(!string.IsNullOrWhiteSpace(name), nameof(name));
	_ectx.CheckParam(!name.Contains(".."), nameof(name));
	// The gymnastics below are meant to deal with bad invocations including absolute paths, etc.
	// That's why we go through it even if DirTemp is null.
	string root = Path.GetFullPath(DirTemp ?? @"x:\dummy");
	string entityPath = Path.Combine(root, dir ?? "", name);
	entityPath = Path.GetFullPath(entityPath);
	string tempPath = Path.Combine(root, PathMap.Count.ToString());
	tempPath = Path.GetFullPath(tempPath);
	string parent = Path.GetDirectoryName(entityPath);
	_ectx.Check(parent != null);
	_ectx.Check(parent.StartsWith(root));
	int ichSplit = root.Length;
	_ectx.Check(entityPath.Length > ichSplit && entityPath[ichSplit] == Path.DirectorySeparatorChar);
	if (createDir && DirTemp != null && parent.Length > ichSplit)
	Directory.CreateDirectory(parent);
	// Get the relative path portion. This is the archive entry name.
	pathEnt = entityPath.Substring(ichSplit + 1);
	_ectx.Check(Utils.Size(pathEnt) > 0);
	_ectx.Check(entityPath == Path.Combine(root, pathEnt));
	// Set pathTemp to non-null iff _dirTemp is non-null.
	pathTemp = DirTemp != null ? tempPath : null;
	pathEnt = NormalizeForArchiveEntry(pathEnt);
	pathTemp = NormalizeForFileSystem(pathTemp);
	}

[jwood803]

@justinormont Apologies, I totally forgot that I had started looking into this.

Looking again and, from what I can tell, it just needs to check if the temp path already exists and, if it does, make sure it is unique?

Right now, I have this added:

if (Directory.Exists(tempPath))
{
   tempPath = Path.Combine(tempPath, Guid.NewGuid().ToString());
}

Though, I'm not sure if this is even the correct way to go about fixing it.

[justinormont]

@jwood803: I think there's still a race condition since there's a delay before the tempPath is used. So two processes could both check and find that their path is ok to use, then both begin using the same folder.

I think you're on the right track adding a GUID. I haven't worked thru the code, but just always adding a GUID should work.

[jwood803]

@justinormont Good point! Always adding the GUID should help solve the issue. PR will come shortly.

[mstfbl]

This issue will automatically be closed by PR #4645 .