dotnet
WPF will break when an exception be throw in the StylusPlugIn
We can write a class that inherits the StylusPlugIn. And this class can get the touch event fast in stylus input thread in the overwrite method, such as OnStylusDown and the OnStylusUp method. The Stylus Input thread is backgroud thread. As we all know, any exception thrown in a background thread will destroy the application. We are hard to catch the background thread and recover this thread. Though we can use AppDomain.CurrentDomain.UnhandledException and add <legacyUnhandledExceptionPolicy enabled="1"/> to app.config to catch it.
The StylusPlugIn can be inherited and we can overwrite the method. And we can not stop our friend add his code in the method, such as OnStylusDown method. But we may not enough careful in our code that the code will throw the exception in unexpected in the OnStylusDown method. And the exception will break the stylus input thread and the WPF application will stop responding the touch. And we can not do something to recover the touch responding. Just like this code. The code will break the Stylus Input thread.
public partial class MainWindow : Window
{
public MainWindow()
{
InitializeComponent();
StylusPlugIns.Add(new Foo());
}
}
public class Foo : StylusPlugIn
{
/// <inheritdoc />
protected override void OnStylusDown(RawStylusInput rawStylusInput)
{
throw new Exception();
}
}
The OnStylusDown running in Stylus Input thread and if some friend throws any exceptions that will break the thread. And the application will stop responding touch.
Of course, the actual situation may be more complicated. Maybe some of my friends just didn't realize that the code he wrote was unstable. But add try catch to all the method is an evil code.
But I do not think we can throw any unintended exceptions in StylusPlugIn.
See StylusPlugIn Class (System.Windows.Input.StylusPlugIns)
If you use a StylusPlugIn inside a control, you should test the plug-in and control extensively to make sure they do not throw any unintended exceptions.
See Intercepting Input from the Stylus
If a StylusPlugIn throws or causes an exception, the application will close. You should thoroughly test controls that consume a StylusPlugIn and only use a control if you are certain the StylusPlugIn will not throw an exception.
Just as the document say, we should make sure the code do not throw any unintended exceptions in StylusPlugIn. But actual we are hard to do it in the complex business.
I think it may be a good way that we can catch all the exceptions in the code that fire the StylusPlugIn method.
We are making a full-screen touch application and the user can only control the machine by touch input. If the WPF application stops responding touch, the user can not do something except reboot.
All the demo code in github
We are making a full-screen touch application and the user can only control the machine by touch input. If the WPF application stops responding touch, the user can not do something except reboot.
How does the app handle this exception differently then doing a try {} catch() ? Or are you suggesting that we just catch and swallow all the errors and don't crash at all?
He's suggesting to reroute any unhandled exceptions from the StylusPlugin call site, so it doesn't terminate the stylus input thread. The exception still surfaces through the global unhandled exception handlers, if the application has no unhandled exception handler (the default) then it will still crash, but if it has the hope is that the stylus input continues to work.
We are making a full-screen touch application and the user can only control the machine by touch input. If the WPF application stops responding touch, the user can not do something except reboot.
How does the app handle this exception differently then doing a
try {} catch()? Or are you suggesting that we just catch and swallow all the errors and don't crash at all?
As a powerfully UI framework, I think WPF should withstand some break action just as throw the exception in StylusPlugin code. We should not swallow any user code error but we should make our framework stabilization.
We should not suppose all the code in StylusPlugin will never throw any exception, though we do enough test work. If the exception occurs in a non-critical business code, we should ensure that the core framework still works. But in fact any code in StylusPlugin, even the edge of the module, will make the application stop responding touch.
I also agree with this view that swallow all the exception is not a good way. But I think the framework should offer some way to recover the Stylus Input thread. We do not care where the exception is thrown and we do not care whether we handle the exception, but we care the application can respond the touch alway.
Just as my words, we are making a full-screen touch application and the user can only control the machine by touch input. In this industry, touch is very important just like the mouse in PC. Can you tolerate any code to easily make the program no longer respond to mouse events by throw an exception?
There has been some discussion on this issue on a PR (start at https://github.com/dotnet/wpf/pull/945#issuecomment-538458809 ) that is worth reading and will provide more context.
In response to https://github.com/dotnet/wpf/pull/945#issuecomment-538505844 by @rladuca
Why not provide a (ThreadStatic) event in StylusPlugin for error trapping and let the application developer decide what to do with them? If the error isn't marked as handled after the event handlers are called, allow the exception to crash the Stylus thread (for compat reasons).
Is this fundamentally different than re-using the Application.DispatcherUnhandledException that PR #945 proposes?
If an exception in a StylusPlugin didn't kill the Stylus thread the rest of the application would, generally, function well. There could be some erroneous behavior that is application specific, but not framework specific. For instance, if the application was using a plugin to limit stylus input to a specific area, then a failure in a plugin could then expand the input area unintentionally.
I might need to take a nap, to me this sounds almost better than allowing apps to crash, even if it's the apps fault. I've never thought that way before, so I might be feeling ill or something 🤣
/cc @dotMorten @lindexi
Is this fundamentally different than re-using the
Application.DispatcherUnhandledExceptionthat PR #945 proposes?
Yes, that's changing the purpose of the mechanism at a framework level, directly contradicting guidance:
If an exception is not handled on either a background UI thread (a thread with its own Dispatcher) or a background worker thread (a thread without a Dispatcher), the exception is not forwarded to the main UI thread.
Also firing an event directly on the Stylus thread allows the developer to recover appropriately at the point of execution of the plugin rather than much later where recovery might not be possible. Note that this is not really something directed at a plugin author, but a plugin user (in the development sense). If the plugin is your own, fix your plugin. If not, you can mitigate the damage it is doing.
I might need to take a nap, to me this sounds almost better than allowing apps to crash, even if it's the apps fault. I've never thought that way before, so I might be feeling ill or something 🤣
I am definitely saying it is better not to crash (it's how I expect all plugin style systems to work), but only if a developer can respond to these errors. The application developer may decide that things are unrecoverable and crash anyway or even forward to the UI thread, but it should be entirely up to them.
Having spent a lot of time digging around the stylus stack for various issues, I am entirely sympathetic to complaints about its design failures.
Cool, yeah I agree with you that using Application.DispatcherUnhandledException would be the wrong thing here after reading that :)
The application developer may decide that things are unrecoverable and crash anyway or even forward to the UI thread, but it should be entirely up to them.
Yup, glad we're on the same page.
@lindexi do you want to propose an API based on what @rladuca has suggested? Note that something like this would need official API review and we're still not really in a place where we are ready to start that, so it would be a few months before any sort of progress is made. I just want to set expectations, so it's ok if you'd rather hand this off :)
IMHO this is an exception thrown by user-code/3rd party library. You can never ever guard completely against this, and I don't think you should. There would be a gazillion places where extension points in an API could cause a non-catchable crash, but if .NET starts guarding against this, it'll be impossible for the user/3rd party vendor ever figure out here's a problem in the first place and address it. Also re-throwing on a different thread would break the callstack.
At least with a crash you should be getting a callstack to the offending piece of code. But if it's a 1st change exception, we might have no clue something isn't working as it should and wondering why something else isn't.
As a component vendor myself, I'd want my users' apps to crash so they can come slap me with a callstack and I can fix it. It's a hell of a lot easier to address, than when they report that "something isn't working quite right but app isn't crashing either".
I agree that, generally, guarding extension points is a fool's errand. That's why I also don't support just eating the exception at a framework level. I want to maintain the current default behavior, but give an application developer an option to recover and workaround problems they encounter with a 3rd party StylusPlugin. That developer can generate telemetry, warn the user, just silently swallow it, disable the offending plugin, however they want to proceed. In fact, they can give you the data you need to debug your component since it is now directly accessible at the point of failure.
EDIT: Or I should say, much closer to the point of failure.
@stevenbrix
@lindexi do you want to propose an API based on what @rladuca has suggested? Note that something like this would need official API review and we're still not really in a place where we are ready to start that, so it would be a few months before any sort of progress is made. I just want to set expectations, so it's ok if you'd rather hand this off :)
Thank you for your understanding and support.
I think it is hard to propose a good API to do it.
I think we should do more design before writing the code. Actually, I do not know what is the expectation and which API can be public. So I list some of my expectation that the new API can do.
-
No one can break the stylus input thread, or the stylus input can work well as the mouse input.
It means that we do not need to do anything to fix the stylus input thread.
-
We can know any exception throw in the stylus input thread.
Maybe it can fire an event just as Application.DispatcherUnhandledException
-
The developers do not need to learn more about it.
The developers are familiar with the API that just like the mouse input API.
To fix another problem, it a good thing to provide an API to re-run the stylus input thread. This problem is sometimes the applications will only get the mouse message in touch device in win7 which system does not install the right USB device program. Because this is the other problem, I should not talk more here.
@rladuca
Why not provide a (ThreadStatic) event in StylusPlugin for error trapping and let the application developer decide what to do with them?
Thank you to the great suggestion.
Could you point how to provide this event? I do know which code can listen to this event. Because one UI thread can build one stylus input thread and all the stylus logic is internal now. We can only use the stylus plugin by UIElement. If we provide this event that means we should public some internal class.
In my opinion, I do not think the developers must decide what to do with them. The mouse and the keyboard input will throw the exception in the main thread and the developers can handle all of them in dispatcher unhandled exception. But why the stylus plugin exception should be handled alone? I think it hard to understand.
Maybe someone will say, "Why I should know more about how to handle the touch exception? Why the touch and mouse input is difference? Why can not I handle all the input exception."
If the error isn't marked as handled after the event handlers are called, allow the exception to crash the Stylus thread (for compat reasons).
I am worried about the developers should learn more about it. Before we use the stylus plugin, we should know the code will run in the stylus input thread which is a background thread and we should know we need to listen to the unhandled exception in stylus input thread and if we do not mark as handled then the stylus input thread will break. Can we break the compat? The default behavior is auto fix the stylus input thread.
@dotMorten
Also re-throwing on a different thread would break the callstack.
Thank you.
I use the ExceptionDispatchInfo to rethrow which can store the call stack. And the developers can handle the exception in dispatcher unhandled exception just like the exception throw in mouse input. And the exception will store the call stack.
See: ExceptionDispatchInfo Class (System.Runtime.ExceptionServices)
The ExceptionDispatchInfo object stores the stack trace information and Watson information that the exception contains at the point where it is captured. The exception can be thrown at another time and possibly on another thread by calling the ExceptionDispatchInfo.Throw method. The exception is thrown as if it had flowed from the point where it was captured to the point where the Throw method is called.
I think we should do more design before writing the code.
@lindexi I totally agree! I didn't mean to ask you to write the feature, rather just come up with the API that you would expect to use. That will help us move the design along.
Actually, I do not know what is the expectation and which API can be public.
There are no expectations, you can come up with an API and then we can discuss from there :). To me, it sounds like you have a pretty good understanding of what the overall requirements of this API are (with the help from @rladuca and @dotMorten). With that being said, if you don't want to that is totally fine. I'm just trying to move this issue along a bit, because I think there is value to add something like this, but we aren't really ready to fully devote our attention to .NET 5 work yet.
@stevenbrix Thank you for your understanding and support.
The #945 will change the behavior but it can fix #1037 simply. Please let me summarize the advantages and disadvantages.
The advantage is that
- Doing as the mouse event and the developer can handle the exception like the mouse event in main thread.
- Unified touch and mouse exception handling.
- The developers need not learn more about StylusPlugin and learn more API
- Keeping the exception call stack and all message by using ExceptionDispatchInfo
- We need not public any internal API and the code is simply that maybe can publish fast and we need not wait for long time. Hope it can fix fast. Because of some of my project will break for this issue.
The disadvantage is that
- Break the compatibility that the unhandled exception will not break the stylus input thread.
- The unhandled exception in the stylus input thread will throw to the main thread. It will change the thread but this behavior can unified the exception handling.
- Guarding exception may make the developer miss the bug in the test.
Thank you and hope the WPF being more powerful and easier use.
@lindexi I think I have a reasonable proposal that would make people happy and pave the way for further improvements. Let me type up a general outline of it and discuss it with the team, then I will post it here.
I do want to set expectations with timing though. This is a 5.0 milestone change, so you'd only see this in daily builds and, eventually, previews for some time. An official release is quite far off. I just want to make sure that is understood.
@lindexi Sorry, I haven't worked on WPF for several years now, I'm not the person who can help move this forward anymore. I don't even have the context of what was being discussed anymore tbh. My last update has been, unfortunately, lost to time and career changes.
@dotnet/wpf-developers to engage here.