-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix plugin exception #3426
Fix plugin exception #3426
Conversation
…roject#3366)" This reverts commit f307a31.
…roject#3366)" This reverts commit f307a31.
Co-authored-by: Christopher Schuchardt <cschuchardt88@gmail.com>
…plugin-exception * 'fix-plugin-exception' of github.com:Jim8y/neo: Update src/Neo/Plugins/UnhandledExceptionPolicy.cs ensure leveldb is not used in multithread env Revert "Revert "Plugin unhandled exception (neo-project#3349)" (neo-project#3366)" # Conflicts: # src/Neo/Ledger/Blockchain.cs # src/Neo/Plugins/UnhandledExceptionPolicy.cs # tests/Neo.UnitTests/Plugins/TestPlugin.cs # tests/Neo.UnitTests/Plugins/UT_Plugin.cs
Co-authored-by: Christopher Schuchardt <cschuchardt88@gmail.com>
@@ -3,7 +3,8 @@ | |||
"Path": "ApplicationLogs_{0}", | |||
"Network": 860833102, | |||
"MaxStackSize": 65535, | |||
"Debug": false | |||
"Debug": false, | |||
"UnhandledExceptionPolicy": "StopPlugin" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better in almost all of them StopNode
by default, is the current behaviour
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nothoing was changed since the last review but the deledage invoke. Different plugin has different behavior on unhandled exception that are precisely set by superboy. Default behavior means nothing here.
Co-authored-by: Shargon <shargon@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing right now.
using System.Collections.Generic; | ||
using System.Collections.Immutable; | ||
using System.Diagnostics; | ||
using System.Linq; | ||
using System.Threading.Tasks; | ||
using System.Runtime.CompilerServices; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What we have this requirement added?
// skip stopped plugin. | ||
if (handler.Target is Plugin { IsStopped: true }) | ||
{ | ||
continue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the stopped plugin keep crashing nothing will be reported?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested with OracleService and StateService on the same node, deployed Oracle functions and invoked. Everything is working even with StopNode as default.
Description
This pr reopens the plugin unhandled exception pr, and fixed the async delegate invocation calling.
Fixes # #3356
Type of change
How Has This Been Tested?
Test Configuration:
Checklist: