After using FileStream.SafeFileHandle, is it safe for me to remove a call to GC.KeepAlive()?

Question

Consider the following code which I recently changed to use FileStream.SafeFileHandle:

public static void FastWrite<T>(FileStream fs, T[] array, int offset, int count) where T: struct
{
    int sizeOfT = Marshal.SizeOf(typeof(T));
    GCHandle gcHandle = GCHandle.Alloc(array, GCHandleType.Pinned);

    try
    {
        uint bytesWritten;
        uint bytesToWrite = (uint)(count * sizeOfT);
        var overlapped = new NativeOverlapped();

        if
        (
            !WriteFile
            (
                fs.SafeFileHandle,
                new IntPtr(gcHandle.AddrOfPinnedObject().ToInt64() + (offset*sizeOfT)),
                bytesToWrite,
                out bytesWritten,
                ref overlapped
            )
        )
        {
            throw new IOException("Unable to write file.", new Win32Exception(Marshal.GetLastWin32Error()));
        }

        Debug.Assert(bytesWritten == bytesToWrite);

        GC.KeepAlive(fs); // <--- Is this really not necessary?
    }

    finally
    {
        gcHandle.Free();
    }
}

[DllImport("kernel32.dll", SetLastError=true)]
[return: MarshalAs(UnmanagedType.Bool)]

private static extern bool WriteFile
(
    SafeFileHandle       hFile,
    IntPtr               lpBuffer,
    uint                 nNumberOfBytesToWrite,
    out uint             lpNumberOfBytesWritten,
    ref NativeOverlapped lpOverlapped
);

I had previously added the GC.KeepAlive(fs) to ensure that the FileStream won't be garbage-collected until after the Windows API WriteFile() call has returned.

However, after changing to use SafeFileHandle Code Analysis now tells me that it isn't necessary with warning CA2004: Remove calls to GC.KeepAlive:

If you are converting to SafeHandle usage, remove all calls to GC.KeepAlive (object).

I've consulted the documentation on FileStream.SafeFileHandle but it is unclear to me if it really is safe for me to remove the call to GC.KeepAlive().

Is it definitely safe to remove it? And am I using it correctly?

Also, can anyone point me at some decent documentation on the use of SafeHandle?

@Damien_The_Unbeliever Heh, thanks for the edit. It was in the title too! — Matthew Watson, Apr 30 '13 at 10:14

score 4 · Accepted Answer · edited May 23 '17 at 12:12

The point of using SafeHandle is that the handle won't be closed while the WriteFile() function is executing. Which is really what you want to achieve here. Do note however that the FileStream object may still be finalized. There are no obvious consequences from that happening in the posted code. So the FxCop warning is appropriate.

Do note the strings attached to using code like this. It is very unlikely to be any faster than FileStream.Write(). But you do add risk by not properly dealing with edge conditions. Including using Overlapped but not appropriately handling overlapped I/O, don't do that. If overlapped I/O is actually intended then check this answer for the way that's optimized in the CLR beyond what GCHandle can do. Have a good look at the Reference Source source code for FileStream, particularly focus on the _isAsync field and the error handling for ERROR_NO_DATA and ERROR_INVALID_HANDLE.

And you'll also see it using SafeFileHandle and not using GC.KeepAlive(), like FxCop demands.

Thanks Hans. This code was originally written years ago to help us to interoperate with legacy C++ code, so it was more focused on that than on speed. However, it *seems* to be a lot faster than using `BinaryWriter` (so much faster in fact that I think I must be doing something wrong, so I'm going to make a new question about that). — Matthew Watson, Apr 30 '13 at 11:11

After using FileStream.SafeFileHandle, is it safe for me to remove a call to GC.KeepAlive()?

1 Answers1