Navigation should always be handled in the ViewModel.
You're on the right track with thinking that the perfect implementation of the MVVM design pattern would mean you could run your application entirely without Views, and you can't do that if your Views control your Navigation.
I usually have an ApplicationViewModel
, or ShellViewModel
, which handles the overall state of my application. This includes the CurrentPage
(which is a ViewModel) and the code for handling ChangePageEvents
. (It's often also used for other application-wide objects such as the CurrentUser, or ErrorMessages too)
So if any ViewModel, anywhere, broadcasts a ChangePageEvent(new SomePageViewModel)
, the ShellViewModel
will pickup that message and switch the CurrentPage
to whatever page was specified in the message.
I actually wrote a blog post about Navigation with MVVM if you're interested
Your code has significant other issues apart from just that. Manually deleting a pointer? Calling a cleanup
function? Owch. Also, as accurately pointed out in the question comment, you don't use RAII for your lock, which is another fairly epic fail and guarantees that when DoSomethingImportant
throws an exception, terrible things happen.
The fact that this multithreaded bug is occurring is just a symptom of the core problem- your code has extremely bad semantics in any threading situation and you're using completely unreliable tools and ex-idioms. If I were you, I'd be amazed that it functions with a single thread, let alone more.
Common Mistake #3 - Even though the objects are reference counted, the
shutdown sequence "releases" it's pointer. But forgets to wait for the
thread that is still running to release it's instance. As such,
components are shutdown cleanly, then spurious or late callbacks are
invoked on an object in an state not expecting any more calls.
The whole point of reference counting is that the thread has already released it's instance. Because if not, then it cannot be destroyed because the thread still has a reference.
Use std::shared_ptr
. When all threads have released (and nobody, therefore, can be calling the function, as they have no pointer to it), then the destructor is called. This is guaranteed safe.
Secondly, use a real threading library, like Intel's Thread Building Blocks or Microsoft's Parallel Patterns Library. Writing your own is time-consuming and unreliable and your code is full of threading details which it doesn't need. Doing your own locks is just as bad as doing your own memory management. They have already implemented many general-purpose very useful threading idioms which work correctly for your use.
Best Answer
Firstly - I'm assuming that the thread running your test has some mechanism which can notify on its own progress somehow (maybe with an
event
ordelegate
?)In the spirit of MVVM, consider a
Model
which represents the state and progress of your test which you can be updated by the test thread.The main purpose of the
Model
would be to remember the state of the UI progress bar (e.g. anint
for a percentage). For example:With the MVVM-based UI frameworks (such as Silverlight, WPF, etc.) you need a way to ensure that any updates affecting the UI state (and therefore the properties on the
Model
) are invoked using theDispatcher
thread (the main/UI thread).(Related: https://stackoverflow.com/questions/4621623/wpf-multithreading-ui-dispatcher-in-mvvm)
You may have as many
ThreadPool
threads as you like, but those threads will need to invoke UI state changes on theDispatcher
thread when you're changing any properties on theModel
, sinceINotifyPropertyChanged
causes the state of the UI to change.Also, When dealing with multiple components of the same time, an
ObservableCollection<>
is a typical choice; where each model could bind to a UI Progress Bar component (e.g. a<ListBox>
). Again, any kind of modifications such asAdd
orRemove
needs to happen on theDispatcher
thread.Here's a possible example for a ViewModel where the action of starting a test thread originates from an
ICommand
(button click perhaps?)Note that
Task.Run
executes its action on a threadpool thread - More info on Stephen Cleary's blog - Creating TasksNone of the above code really takes advantage of the full capabilities
async
/await
orTask
.Among the reasons to use
Task
instead of starting a threadpool thread directly include:CancellationTokenSource
)Task.WhenAny
andTask.Delay
) )Task.TrySetResult
).For these things you might find Stephen's
AsyncCommand
to be useful - Async Programming : Patterns for Asynchronous MVVM Applications: CommandsThe
AsyncCommand
may help simplify the task of 'chaining' multiple dependent tests. For example, a list ofTestModel
objects.This snippet assumes that
RunTestsAsync
happens inside anAsyncCommand
, eachTask
is executed as the previous task completes, but theasync
/await
specifiers prevent the Dispatcher thread from being blocked while eachTask
awaits completion:Note: Code snippets above aren't tested or checked in a compiler, but the concepts are more important than the actual code.
The
Task
,async
andawait
features of C# can be a little mindblowing the first time you see them. Some extra reading is highly recommended, as well as trying examples in a test app to get comfortable with how they work in practice.The most important thing to realise is that while a traditional
Thread.Wait
will block the UI thread, theawait
keyword will not block the UI thread, and is a very elegant way of handling long-running tasks inside a Button click command or other GUI event handler.