Critique this code: The memory mapped file

time to read 4 min | 654 words

We got an error in the following code, in production. We are trying hard to make sure that we have good errors, which allows us to troubleshoot things easily.

In this case, the code… wasn’t very helpful about it. Why? Take a look at the code, I’ll explain why below…

public CodecIndexInput(FileInfo file, Func<Stream, Stream> applyCodecs)
{
    try
    {
        this.file = file;
        this.applyCodecs = applyCodecs;

        fileHandle = Win32NativeFileMethods.CreateFile(file.FullName,
            Win32NativeFileAccess.GenericRead,
            Win32NativeFileShare.Read | Win32NativeFileShare.Write | Win32NativeFileShare.Delete,
            IntPtr.Zero,
            Win32NativeFileCreationDisposition.OpenExisting,
            Win32NativeFileAttributes.RandomAccess,
            IntPtr.Zero);

        if (fileHandle.IsInvalid)
        {
            const int ERROR_FILE_NOT_FOUND = 2;
            if (Marshal.GetLastWin32Error() == ERROR_FILE_NOT_FOUND)
                throw new FileNotFoundException(file.FullName);
            throw new Win32Exception();
        }

        mmf = Win32MemoryMapNativeMethods.CreateFileMapping(fileHandle.DangerousGetHandle(), IntPtr.Zero, Win32MemoryMapNativeMethods.FileMapProtection.PageReadonly,
            0, 0, null);
        if (mmf == IntPtr.Zero)
        {
            throw new Win32Exception();
        }

        basePtr = Win32MemoryMapNativeMethods.MapViewOfFileEx(mmf,
            Win32MemoryMapNativeMethods.NativeFileMapAccessType.Read,
            0, 0, UIntPtr.Zero, null);
        if (basePtr == null)
            throw new Win32Exception();

        stream = applyCodecs(new MmapStream(basePtr, file.Length));
    }
    catch (Exception)
    {
        Dispose(false);
        throw;
    }
}

Did you see it?

This code has multiple locations in which it can throw Win32Exception. The problem with that is that Win32Exception in this mode is pretty much just a code, and we have multiple locations inside this method that can thrown.

When that happens, if we don’t have the PDB files deployed, we have no way of knowing, just from the stack trace (without line numbers), which of the method calls had caused the error. That is going to lead to some confusion after the fact.

We solved this by adding description text for each of the options, including additional information that will let us know what is going on. In particular, we also included not only the operation that failed, but even more important, we included the file that failed.