+972 52-548-6969
Talk about nasty bugs
time to read 2 min | 305 words
Let us see how many of you can figure this one out.
Here is the code:
[DllImport("kernel32.dll", SetLastError = true, CharSet = CharSet.Auto)] [return: MarshalAs(UnmanagedType.Bool)] static extern bool CopyFileEx(string lpExistingFileName, string lpNewFileName, IntPtr lpProgressRoutine, IntPtr lpData, ref Int32 pbCancel, CopyFileFlags dwCopyFlags); public void CopyFileWithProgressReports() { var lpProgressRoutineIntPtr = Marshal.GetFunctionPointerForDelegate(new CopyProgressRoutine(LpProgressRoutine)); int pbCancel = 0; CopyFileEx(hugeFile, hugeFile + ".new", lpProgressRoutineIntPtr, IntPtr.Zero, ref pbCancel, CopyFileFlags.COPY_FILE_RESTARTABLE); } private static CopyProgressResult LpProgressRoutine(long totalFileSize, long totalBytesTransferred, long streamSize, long streamBytesTransferred, uint dwStreamNumber, CopyProgressCallbackReason dwCallbackReason, IntPtr hSourceFile, IntPtr hDestinationFile, IntPtr lpData) { Console.WriteLine("{0:#,#} / {1:#,#}", totalBytesTransferred, totalFileSize); return CopyProgressResult.PROGRESS_CONTINUE; }
This code will run perfectly if you execute it in a single threaded fashion.
But, if you run it on a background thread and continue to do additional operations (just running it on a background thread work) that has nothing to do with the file system, it will crash, sometimes with a null reference exception, sometimes with attempt to write to protected memory, etc.
There is a very subtle bug here, can you figure out what it is?
Comments
static & threading = trouble
I don't know how it related to threading but one of the reasons can be that you don't keep reference to the delegate.
If you were updating a graphical progress bar, the call to update it would have to be invoked to avoid cross-threading issues. Does that also apply when writing to the console?
Seconding that not keeping a reference to the delegate to prevent it from being garbage collected before CopyFileEx() is done with it could be a potential problem. I've been burned by that before myself.
By the way, is there really a need to manually call Marshal.GetFunctionPointerForDelegate()? I'm pretty sure I've been able to specify an actual delegate type in a P/Invoke signature and the framework handled the conversion on its own.
Could it be that you've got SetLastError set to true, are not calling GetLastWin32Error() anywhere, and when more than one thread is running there are errors trying to be set?
You're not using a SafeHandle?
OmariO & Michael,
Wow, that was fast.
Yep that is the problem.
Dude! You blog so much that I wonder how you ever get work done.
The reason why it only crashes if there is additional work done is that the garbage collector only runs if an allocation occurs.
I've never used Marshall methods. I take it that the GetFunctionPointerForDelegate method causes the callback method to be invoked in a thread-safe manner?
What is the solution? Make var lpProgressRoutineIntPtr a class variable instead of a local?
I suspect that if you were to run this on Mono, you WOULDN'T see this problem because it employs a conservative GC.
Hmmm, looks familiar ;-)
Is this actually broken with a static method as well as member methods? What does reflector say the actual code looks like?
Yes, it does.
The only important thing is that it isn't rooted
This is actally in the docs:
You must manually keep the delegate from being collected by the garbage collector from managed code. The garbage collector does not track reference to unmanaged code.
See: msdn.microsoft.com/.../...npointerfordelegate.aspx
Comment preview