1

Note: Question has been reworded to distinguish it more clearly from: this

I have some code firing from windows shell context menu, that brings up a form to display progress to the user.

The code implementing as follows:

The explorer interface:

public void InvokeCommand(IntPtr pici)
{
    CMINVOKECOMMANDINFO ici = (CMINVOKECOMMANDINFO)Marshal.PtrToStructure(
        pici, typeof(CMINVOKECOMMANDINFO));

    // Identify what was clicked etc... 
    TransferSalesProject(ici.hwnd);
}

[ComImport(), InterfaceType(ComInterfaceType.InterfaceIsIUnknown)]
[Guid("000214e4-0000-0000-c000-000000000046")]
internal interface IContextMenu
{
    void InvokeCommand(IntPtr pici);
}

[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)]
internal struct CMINVOKECOMMANDINFO
{
    public uint cbSize;
    public CMIC fMask;
    public IntPtr hwnd;
    public IntPtr verb;
    [MarshalAs(UnmanagedType.LPStr)]
    public string parameters;
    [MarshalAs(UnmanagedType.LPStr)]
    public string directory;
    public int nShow;
    public uint dwHotKey;
    public IntPtr hIcon;
}

The code calling the form:

async void TransferSalesProject(IntPtr hWnd)
{
    try
    {
        string basePath = @"C:\Folder1";
        string destFolder = @"P:\Folder1"

        // Check that project hasn't already been transferred.
        if (!Directory.Exists(destFolder))
        {
            // Move folder to projects current folder.
            DirectoryMove dirMove = new DirectoryMove(basePath, destFolder);
            dirMove.ShowDialog();
        }
        else
        {
            MessageBox.Show("Destination Folder already exists.", "Error");
        }
    }
    catch (Exception e)
    {
        MessageBox.Show(e.Message, "Exception");
    }
}

The code in the form:

public partial class DirectoryMove : Form
{
public string BaseFolder { get; set; }
public string DestFolder { get; set; }
public Progress<string[]> progress { get; set; }

public DirectoryMove(string baseFolder, string destFolder)
{
    InitializeComponent();
    this.BaseFolder = baseFolder;
    this.DestFolder = destFolder;
    progress = new Progress<string[]>();
    progress.ProgressChanged += (sender, progressValue) =>
    {
        if (!this.IsDisposed && this.InvokeRequired)
        {
            this.Invoke((MethodInvoker)delegate
            {
                this.BringToFront();
                this.progressBar.PerformStep();
                this.Text = progressValue[0];
                this.progressText.Text = progressValue[1];
            });
        }
        else
        {
            this.BringToFront();
            this.progressBar.PerformStep();
            this.Text = progressValue[0];
            this.progressText.Text = progressValue[1];
        }
    };
}

private async void DirectoryMove_Load(object sender, EventArgs e)
{
    try
    {
        await Task.Run(() =>
            {
                try
                {
                    DirectoryCopy(BaseFolder, DestFolder);
                }
                catch
                {
                    throw;
                }
            });
    }
    catch (Exception exception)
    {
        MessageBox.Show(this, exception.Message, "Exception");
    }
    finally
    {
        this.Close();
    }
}

private void DirectoryCopy(string sourceDirName, string destDirName, bool copySubDirs = true)
{
    // Get the subdirectories for the specified directory.
    DirectoryInfo dir = new DirectoryInfo(sourceDirName);

    DirectoryInfo[] dirs = dir.GetDirectories();
    // If the destination directory doesn't exist, create it.
    if (!Directory.Exists(destDirName))
    {
        Directory.CreateDirectory(destDirName);
    }


    // Get the files in the directory and copy them to the new location.
    FileInfo[] files = dir.GetFiles();
    foreach (FileInfo file in files)
    {

        string temppath = Path.Combine(destDirName, file.Name);
        ((IProgress<string[]>)progress).Report(new string[] { "Copying Files", file.FullName });

        file.CopyTo(temppath, false);
    }


    // If copying subdirectories, copy them and their contents to new location.
    if (copySubDirs)
    {
        foreach (DirectoryInfo subdir in dirs)
        {
            string temppath = Path.Combine(destDirName, subdir.Name);
            DirectoryCopy(subdir.FullName, temppath, copySubDirs);
        }
    }
}

I seem to be getting a InvalidOperationException when I fire this.Close() stating (DirectoryMove is the name of my form):

Cross-thread operation not valid: Control 'DirectoryMove' accessed from a thread other than the thread it was created on.

However this exception only comes up in debug, and seems to be inconsistent in firing. It seems to mostly come up when I trigger other context menu item click events, then fire this one.

I get this off the stack:

at System.Windows.Forms.Control.get_Handle()

As the error suggests, its caused because I am trying to access the form from a non-UI thread. But the form_load event seems like it should run on the UI thread. Philippe Paré has suggested to Invoke my form close, and this seems to have fixed it.

But it leads to the question, why is my form not running on the UI thread. The only reason I can see at the moment is that explorer is doing some sort of weird threading itself, when I call the form. Phillip has suggested it is due to the context of my code changing when I fire Task.Run, but I have tried to emulate this issue with simple form calling a form with the same structure, and haven't been able to replicate the issue.

Anyone who can explain why would be appreciated.

Community
  • 1
  • 1
  • `this.Close()` is a UI method. You have to be on the main thread to invoke it. In the first bit of code you have, you call `this.Invoke` with a delegate in there. Do the same for `this.Close()` and you'll be good! – Philippe Paré Mar 21 '16 at 01:57
  • 1
    @PhilippeParé I thought `Form_Load` is fired in the UI thread, so it doesn't make sense to invoke it. Its the same reason I don't `Invoke` my `MessageBox.Show()`. – Mayura Vivekananda Mar 21 '16 at 02:20
  • It certainly appears that `this.Close` is running on the UI thread. But are you certain that the `DirectoryMove` window was initially constructed on the UI thread? – Andrew Shepherd Mar 21 '16 at 03:47
  • Yes, I call the form via `DirectoryMove dirMove = new DirectoryMove(args);` then `dirMove.ShowDialog();`. So the only point any new threads are used, is once I am executing code inside the `Load`. – Mayura Vivekananda Mar 21 '16 at 03:50
  • You can use `ContinueWith(t => { this.Close(); }, TaskScheduler.FromCurrentSynchronizationContext())`. This will run on the Current UI Thread. – Bon Macalindong Mar 21 '16 at 03:54
  • @BonMacalindong As mentioned, I believe I am on the current UI thread, so surely I do not need to do any trickery around my `this.Close()`? – Mayura Vivekananda Mar 21 '16 at 03:55
  • Yes. Though the code I mentioned above will execute on the UI thread, it's synchronized with the task thus you will not encounter the cross thread error. – Bon Macalindong Mar 21 '16 at 04:00
  • If you are getting that exception, then you are _not_ executing the code that accesses the `DirectoryMove` object on the same thread that owns that object. There is not enough detail in your question to justify a claim that the question is not a duplicate of the standard technique of using `Control.Invoke()` to deal with the cross-thread exception. You haven't even shown us what `DirectoryMove` is! If you believe your question is not a duplicate, then please fix the question so that it includes a good [mcve] and a clear explanation of how that [mcve] shows that your question isn't a duplicate. – Peter Duniho Mar 21 '16 at 04:02
  • @PeterDuniho DirectoryMove is the name of my form, I forgot to mention it. At this point my question is as much why is `Close()` firing from a non-UI thread, because it should be executing on the UI thread. I am unsure how much more code to attach, because I can't see any real advantage to including all the framework around the `Form`, since it seems more likely to generate a code wall. I am really struggling to see how my question is a duplicate, since the linked question is a generic, "I have a form element that needs to be updated from another thread". While I am on the main UI thread. – Mayura Vivekananda Mar 21 '16 at 04:37
  • _"I am on the main UI thread"_ -- as I already wrote, you cannot possibly be on the same thread that owns the object you're trying to access, because _failing_ to meet that requirement is exactly why that exception is thrown. Please read [mcve] to understand what kind of code example is needed. No one wants to see _all_ of your code; you need to create a _minimal_ example that reproduces the problem you are seeing. Then there won't be any code other than what's _actually needed to reproduce the problem_. – Peter Duniho Mar 21 '16 at 04:43
  • The problem is my code is executing out of explorer via a shell context menu, so there is a bit of backend code to allow the shell context to work. I will try come up with a minimal example tomorrow, but I feel I need to link the library used to add into the shell to achieve this error. – Mayura Vivekananda Mar 21 '16 at 04:53
  • @MayuraVivekananda Sorry about the late response, but the problem is: You `await` your `Task.Run`, then expect the execution to be on the same thread. The thing with await, when the control is returned (the await is done), the execution stays on that other thread. There are 3 things you can do. `.ContinueWith(()=>{})` after your `Task.Run`, Invoke it using `this.Invoke`, or use `.ConfigureAsync(true)` to force it to come back on the current thread after execution. Try that out, tell me how it goes :) – Philippe Paré Mar 21 '16 at 12:31
  • instead of do Invoke in event of control, your should call it in any operation with user UI. do it before, not in the middle. – Alexey Obukhov Mar 21 '16 at 12:50
  • @PhilippeParé I put `.ContinueWith(t => { this.Close(); }, TaskScheduler.FromCurrentSynchronizationContext());` and it seems to be working when I am outside the folder I wish to move, but it fails when I am inside the folder. I tried Invoke, and that seems to solve it. This all has lead me to believe that explorer has some sort of multi-threading. – Mayura Vivekananda Mar 21 '16 at 22:22
  • @PeterDuniho Has the questions context changed enough to no longer be marked a duplicate? Or is there no point reopening this because it has been solved, and is more about an explanation now. Thanks. – Mayura Vivekananda Mar 22 '16 at 04:12
  • The need to call `Invoke()` proves your question was correctly marked a duplicate. As for the specific reason you are in the wrong thread, that is almost certainly _also_ a duplicate, as there have already been many people asking the same basic question on SO since `async`/`await` was introduced in C#. I see no need to reopen this question; all of the information it could solicit is already on Stack Overflow, waiting for any interested person to do a search and find it. – Peter Duniho Mar 22 '16 at 06:07
  • @PeterDuniho I have to disagree with the duplicate, because the linked question is a case of problem A happens in situation A, so can be fixed by solution A. While mine is more like problem A happens in situation B, but is fixed by solution A. As [this](http://stackoverflow.com/questions/28009151/how-does-running-several-tasks-asynchronously-on-ui-thread-using-async-await-wor) shows, I should surely be returning to UI context after I do my `await Task.Run()`. But the fact that I am not consistently returning to UI context means something funny is happening. – Mayura Vivekananda Mar 22 '16 at 08:30
  • However I do believe my problem is an edge case, so probably not worth reopening. But from my searching (maybe its just poor), I can't find any question that covers the context not coming back to the UI unless you do a `.ConfigureAwait(false)` – Mayura Vivekananda Mar 22 '16 at 08:35
  • I agree with you @PeterDuniho, the "duplicate answer" is not exactly the same, your situation is different enough that I think it shouldn't be marked as duplication. – Philippe Paré Mar 22 '16 at 12:30

0 Answers0