0

I have C style Dll, which I have to use in C#. I have created a Pinvoke wrapper for the library. Everything is working fine, but one of the C functions accepts a function pointer (Callback). So, in my C# wrapper which is a static class I added following code:

[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
public delegate void CallbackType(uint status);

/* To prevent garbage collection of delegate, need to keep a reference */
static CallbackType privCallback;

[DllImport(DLL_FILE_NAME, CharSet = CharSet.Unicode, CallingConvention = CallingConvention.Cdecl)]
 public static extern int SetCallback(uint handle, CallbackType callback);

public static int SetMyCallback(uint handle, CallbackType callback)
{
    return SetCallback(handle, callback);
}

Now, in my C# Form class,

private void RefreshCallback(uint status)
{
    this.statusLabel.Text = "Status Code: "+ status.toString();
}

private void myBtn_Click(object sender, EventArgs e)
{
    int status, handle = 0;
    ...
    status = MyDllWrapper.SetMyCallback(handle, RefreshCallback);
    ...
}

The moment my callback is invoked, i get an error:

Cross-thread operation not valid

I want the Callback function to run in the UI thread. What changes should I make in my wrapper to achieve this.

I want to keep the Form class clean and wrapper class easy to use for end users. So, if complex code is required to handle this situation I would like to handle that in the wrapper so that end user can easily use my dll.

Also, will the static instance of delegate inside my static wrapper keep it from garbage collection.

adnan kamili
  • 8,967
  • 7
  • 65
  • 125
  • You need exactly the same code as for managed callback to access UI on main thread Invoke/InvokeRequired (which you obviously know) - please clarify why that is not enough/what exactly you are looking for. – Alexei Levenkov Oct 19 '15 at 18:19
  • @AlexeiLevenkov Some event delegate system within wrapper to which functions can subscribe – adnan kamili Oct 19 '15 at 19:32
  • I still don't get how this is specific to native interop. `RefreshCallback` will have exactly the same problem whether it called from native or managed code if it is called on non-UI thread. You can force callback to be run on UI thread as Ben Voigt [suggested](http://stackoverflow.com/a/33221336/477420) but that would significantly limit flexibility in writing callback (since almost everyone already knows how to handle cases when code called on non-UI thread - http://stackoverflow.com/questions/5868783/solve-a-cross-threading-exception-in-winforms)) – Alexei Levenkov Oct 19 '15 at 19:50
  • @AlexeiLevenkov I have a Linux background (C/C++), haven;t used C# much. I didn't know if it is too obvious. – adnan kamili Oct 19 '15 at 19:56

1 Answers1

2

You can (mostly) detect whether the callback is going to GUI code, and automatically perform it from the thread associated with the UI code.

public static int SetMyCallback(uint handle, CallbackType callback)
{
    var callbackB = callback;
    var syncTarget = callback.Target as System.Windows.Forms.Control;
    if (syncTarget != null)
        callbackB = (v) => syncTarget.Invoke(callback, new object[] { v });
    return SetCallback(handle, callbackB);
}

Another option would be to create a SynchronizationContext based on the thread used to call SetMyCallback, and then make the call using that context.

Ben Voigt
  • 277,958
  • 43
  • 419
  • 720
  • I tried your solution but when callback is invoked the program crashes with error: An unhandled exception of type 'System.StackOverflowException' occurred in System.Windows.Forms.dll – adnan kamili Oct 19 '15 at 18:31
  • @adnankamili: Yeah, I didn't notice the variable was getting lifted into the closure, and calling itself instead of the original `callback` parameter. This should fix that. – Ben Voigt Oct 19 '15 at 18:34
  • Thanks! it worked. I didn't get why it failed, can you explain more. What is more intuitive name for callbackB? You can (mostly) detect whether the callback is going to GUI code? Won't this work in every case? – adnan kamili Oct 19 '15 at 18:49
  • @adnankamili: When the anonymous delegate used `Invoke` to call the callback on the UI thread, it looked up the variable `callback`, and found, not the parameter passed in, which `callback` held at the moment the delegate was created, but the value of `callback` at the time the call happened, which was overwritten by the anonymous delegate itself. – Ben Voigt Oct 19 '15 at 19:12
  • @adnankamili: `callbackB` was introduced to avoid overwriting the original value of `callback`. By the end, `callbackB` is a "safe" link to the callback, either direct if it wasn't going to a UI object, or via a `Invoke` wrapper. Maybe you could name it `wrappedCallback`. – Ben Voigt Oct 19 '15 at 19:13
  • @adnankamili: And the "(mostly)" is referring to the fact that a method might use GUI calls, without actually being a member of a GUI class. Closures inside GUI classes are likely to cause trouble, because the compiler creates a new class for the variable capture, and this internally-created class will fail the "is-a `System.Windows.Forms.Control`" test – Ben Voigt Oct 19 '15 at 19:15
  • Thanks a lot! So what would be the fix for System.Windows.Forms.Control" test fail. Will the program crash? – adnan kamili Oct 19 '15 at 19:18
  • @adnankamili: You'd get the same "cross-thread GUI access" exception, and it can't be fixed in the same manner, because I'm getting `Invoke` from that object associated with the delegate. The bit at the end about `SynchronizationContext` should still work, though. Or you could have the caller pass in the `Form`/`Control` instance as an extra parameter. – Ben Voigt Oct 19 '15 at 19:21
  • This method is so beautiful but has a serious limitation. Can't use this method while distributing the library as end user can use it in any manner. – adnan kamili Oct 19 '15 at 19:27
  • @adnankamili: So either have the user provide the GUI object as a second parameter, or [use `SynchronizationContext`](http://stackoverflow.com/q/1949789/103167). – Ben Voigt Oct 19 '15 at 19:29
  • Can't we add some code to C# RefreshCallback(uint status) callback to fix the loop hole? – adnan kamili Oct 19 '15 at 19:45
  • @adnankamili: I thought that was part of the user-supplied code that has to be kept simple? – Ben Voigt Oct 19 '15 at 19:48
  • Yes, it is but SynchronizationContext seems a bit overkill – adnan kamili Oct 19 '15 at 19:52