6

This is related to my other question How to cancel background printing.

I am trying to better understand the CancellationTokenSource model and how to use it across thread boundaries.

I have a main window (on the UI thread) where the code behind does:

 public MainWindow()
        {
            InitializeComponent();

            Loaded += (s, e) => {
                DataContext = new MainWindowViewModel();
                Closing += ((MainWindowViewModel)DataContext).MainWindow_Closing;

            };
        }

which correctly calls the CloseWindow code when it is closed:

 private void CloseWindow(IClosable window)
        {
            if (window != null)
            {
                windowClosingCTS.Cancel();
                window.Close();
            }
        }

With the selection of a menu item, a second window is created on a background thread:

    // Print Preview
    public static void PrintPreview(FixedDocument fixeddocument, CancellationToken ct)
    {
        // Was cancellation already requested? 
        if (ct.IsCancellationRequested)
              ct.ThrowIfCancellationRequested();

               ............................... 

            // Use my custom document viewer (the print button is removed).
            var previewWindow = new PrintPreview(fixedDocumentSequence);

            //Register the cancellation procedure with the cancellation token
            ct.Register(() => 
                   previewWindow.Close() 
            );

            previewWindow.ShowDialog();

        }
    }

In the MainWindowViewModel (on the UI thread), I put:

public CancellationTokenSource windowClosingCTS { get; set; }

With its constructor of:

    // Constructor
    public MainMenu()
    {
        readers = new List<Reader>();
        CloseWindowCommand = new RelayCommand<IClosable>(this.CloseWindow);
        windowClosingCTS = new CancellationTokenSource();
    }

Now my problem. When closing the MainWindow on the UI thread, windowClosingCTS.Cancel() causes an immediate call to the delegate registered with ct, i.e. previewWindow.Close() is called. This now throws immediately back to the " If (Windows != null) with:

"The calling thread cannot access this object because a different thread owns it."

So what am I doing wrong?

Community
  • 1
  • 1
Alan Wayne
  • 5,122
  • 10
  • 52
  • 95
  • I'm assuming you're using WPF? – d.moncada Dec 07 '16 at 06:04
  • You shouldn't be using multi-threading at all in this process. You aren't CPU bound. Stick to using asynchronous single threaded. – Aron Dec 07 '16 at 06:04
  • @d.moncada yes MVVM. – Alan Wayne Dec 07 '16 at 06:04
  • @Aron ah...you lost me... – Alan Wayne Dec 07 '16 at 06:05
  • "With the selection of a menu item, a second window is created on a background thread:" is where you are going wrong. Although I can't see where you are creating a background thread, since you haven't posted all your code. Do all your work on one UI thread. – Aron Dec 07 '16 at 06:07
  • 1
    "a second window is created on a background thread:" why are you using a background thread? – d.moncada Dec 07 '16 at 06:07
  • @d.moncada I suspect that the OP is using "ShowDialog" to scope the lifetime of the window to the print task, this however is not MVVM. The MVPVM solution to this problem is to create a "Printing Task" which is an awaitable method call (which may show a window). – Aron Dec 07 '16 at 06:09
  • @Aron The full code is in my other question referenced at the top. Since printing and printpreview can take several minutes, I'm attempting to put these on a separate thread and not block the UI thread at all. All works until I close the application without first closing the window on the background thread. Attempting to use CancellationTokenSource to close any background thread before the main application shuts down. I'm attempting to learn new tricks... – Alan Wayne Dec 07 '16 at 06:11
  • @d.moncada I'm using a separate thread to avoid blocking the ui thread--since no user input is needed and printing/printpreview can take serveral minutes to complete. – Alan Wayne Dec 07 '16 at 06:17
  • @AlanWayne You shouldn't be using a background thread to avoid blocking the UI thread. It is extremely hard to do this correctly. You should be using asynchronous callback code (async/await). – Aron Dec 07 '16 at 06:22

2 Answers2

5

Your problem is that your preview window runs on another thread. When you trigger cancellation, you execute the registered action of the cancellation token on that thread, not on the thread your preview is running on.

The gold standard in these cases is to not use two UI threads. This will usually cause trouble and the work you need to handle them is usually not worth it.

If you want to stay with your solution or if you want to trigger cancellation from a background thread, you have to marshal your close operation to the thread your window is opened in:

Action closeAction = () => previewWindow.Close();
previewWindow.Dispatcher.Invoke(closeAction);
Sefe
  • 13,731
  • 5
  • 42
  • 55
  • That worked perfectly. So I guess I am using the cancellationtoken correctly. Since it has been pointed out to me (by multiple people) to use only one thread, I can't see how to do this without blocking. The creation of my fixed document uses Measure and Layout of many framework elements from several thousand records on a network server. The framework elements require an STA thread. TASK uses a MTA thread, hence the two don't mix (and why I used the code in my other question). I'm curious how others have approached this problem. Thanks. Dispatcher.Invoke answered my questions. – Alan Wayne Dec 07 '16 at 06:51
  • 1
    @AlanWayne Task doesn't have anything to do with threads. Task most certainly does not use an MTA thread. `Dispatcher.Invoke` is the "correct" way to do this back in .net 4.5. But we are onto .net 4.6.2, times have changed. – Aron Dec 07 '16 at 07:15
  • 2
    @AlanWayne: The solution is usually not to start a second UI thread, but a non-UI worker thread. Usually, accessing data from UI elements is not performance critical, so the best way to go about it is to collect the data from your UI elements first synchronously, start a worker task and then marshal the result back to the UI thread to display the information. What you can also do is use async/await in your calculation methods. When you have methods that don't support async/await (like the XPS write method in your other question), you use a `TaskCompletionSource` (check my answer there). – Sefe Dec 07 '16 at 08:58
  • 1
    @AlanWayne: How does your solution work out in practie? Because what you are doing is not to cancel the document creation, but you just don't display the preview window. So the document creation will continue to run in the background and not be cancelled. You might want to check out my reply to your other question if you want to also cancel the printing task. – Sefe Dec 17 '16 at 11:48
3

The problem with your code is

With the selection of a menu item, a second window is created on a background thread:

// Print Preview
public static void PrintPreview(FixedDocument fixeddocument, CancellationToken ct)
{
    // Was cancellation already requested? 
    if (ct.IsCancellationRequested)
          ct.ThrowIfCancellationRequested();

           ............................... 

        // Use my custom document viewer (the print button is removed).
        var previewWindow = new PrintPreview(fixedDocumentSequence);

        //Register the cancellation procedure with the cancellation token
        ct.Register(() => 
               previewWindow.Close() 
        );

        previewWindow.ShowDialog();

    }
}

And what I presume to be

Task.Run(() => PrintPreview(foo, cancel));

The correct solution is to do everything on a single thread.

public static Task<bool> PrintPreview(FixedDocument fixeddocument, CancellationToken ct)
{
    var tcs = new TaskCompletionSource<bool>();
    // Was cancellation already requested? 
    if (ct.IsCancellationRequested)
          tcs.SetResult(false);
    else
    {
        // Use my custom document viewer (the print button is removed).
        var previewWindow = new PrintPreview(fixedDocumentSequence);

        //Register the cancellation procedure with the cancellation token
        ct.Register(() => previewWindow.Close());



        previewWindow.Closed += (o, e) =>
        {
             var result = previewWindow.DialogResult;
             if (result.HasValue)
                 tcs.SetResult(result.Value);
             else
                 tcs.SetResult(false);
         }
         previewWindow.Show();
    }

    return tcs.Task;
}

Then call

 var shouldPrint = await PrintPreview(foo, cancel);
 if (shouldPrint)
     await PrintAsync(foo);
Aron
  • 15,464
  • 3
  • 31
  • 64
  • I don't see how this will run asyncronously. Also, when you set the result if the TCS on one occasion to a DialogResult and in another to a bool this will cause trouble. – Sefe Dec 07 '16 at 06:26
  • @Aron I initially did do everything on the same thread, but the creation of the fixeddocument requires extensive database useage with creation of framework elements. Even doing this asynchronously, by and of itself, was taking several minutes during which time the UI thread was effectively blocked. Putting all the creation, printing and printpreview on a separate thread actually works great without blocking--until I try and cancel it. – Alan Wayne Dec 07 '16 at 06:35
  • @AlanWayne Database access should not be blocking the UI thread. Assuming you are using async all the way down. DB access is latency/bandwidth limited and not CPU limited. You should be able to use async Database calls. – Aron Dec 07 '16 at 06:53
  • 1
    @Sefe The point is that the OP seems to want to use `ShowDialog` which seems to want to block some processing, whilst the User interacts with the UI. Additionally, the OP is using WPF, where `Windows.DialogResult` is of type `bool?`. – Aron Dec 07 '16 at 06:56
  • @Aron The problem is not with the database calls, but I am using many differenet xaml framework elements to build the fixedpage from several thousand records. It can take several minutes to build the fixedpages used for display. Building the framework elements and the fixedpages for long reports was effectively blocking the mainthread. – Alan Wayne Dec 07 '16 at 07:36