-
Notifications
You must be signed in to change notification settings - Fork 77
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
Invocations refactoring #85
base: main
Are you sure you want to change the base?
Changes from all commits
90a1f51
b72dfaf
cac578c
54a8ed0
f3e1fee
ad8187f
5be3b5b
1d4a9c4
d8ef7b8
3226be3
e9ee3d0
5db72cd
0aec667
e1f7852
ae0eb0f
29b6a90
c594ffc
45b3c96
3f9b165
c6f1148
e2cf427
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,8 +32,7 @@ | |
using System.Reflection.Emit; | ||
using System.Collections.Generic; | ||
using System.Runtime.CompilerServices; | ||
using System.Threading; | ||
|
||
using System.Threading.Tasks; | ||
using Mono.Debugger.Soft; | ||
using Mono.Debugging.Backend; | ||
using Mono.Debugging.Evaluation; | ||
|
@@ -2145,100 +2144,112 @@ static string MirrorStringToString (EvaluationContext ctx, StringMirror mirror) | |
} | ||
} | ||
|
||
class MethodCall: AsyncOperation | ||
internal class SoftOperationResult : OperationResult<Value> | ||
{ | ||
readonly InvokeOptions options = InvokeOptions.DisableBreakpoints | InvokeOptions.SingleThreaded; | ||
public SoftOperationResult (Value result, bool resultIsException, Value[] outArgs) : base (result, resultIsException) | ||
{ | ||
OutArgs = outArgs; | ||
} | ||
|
||
readonly ManualResetEvent shutdownEvent = new ManualResetEvent (false); | ||
public Value[] OutArgs { get; private set; } | ||
} | ||
|
||
internal class SoftMethodCall: AsyncOperationBase<Value> | ||
{ | ||
readonly InvokeOptions options = InvokeOptions.DisableBreakpoints | InvokeOptions.SingleThreaded; | ||
readonly SoftEvaluationContext ctx; | ||
readonly MethodMirror function; | ||
readonly Value[] args; | ||
readonly object obj; | ||
IAsyncResult handle; | ||
Exception exception; | ||
IInvokeResult result; | ||
|
||
public MethodCall (SoftEvaluationContext ctx, MethodMirror function, object obj, Value[] args, bool enableOutArgs) | ||
readonly IInvocableMethodOwnerMirror obj; | ||
IInvokeAsyncResult invokeAsyncResult; | ||
|
||
public SoftMethodCall (SoftEvaluationContext ctx, MethodMirror function, IInvocableMethodOwnerMirror obj, Value[] args, bool enableOutArgs) | ||
{ | ||
this.ctx = ctx; | ||
this.function = function; | ||
this.obj = obj; | ||
this.args = args; | ||
if (enableOutArgs) { | ||
this.options |= InvokeOptions.ReturnOutArgs; | ||
options |= InvokeOptions.ReturnOutArgs; | ||
} | ||
if (function.VirtualMachine.Version.AtLeast (2, 40)) { | ||
this.options |= InvokeOptions.Virtual; | ||
options |= InvokeOptions.Virtual; | ||
} | ||
} | ||
|
||
public override string Description { | ||
get { | ||
return function.DeclaringType.FullName + "." + function.Name; | ||
} | ||
} | ||
|
||
public override void Invoke () | ||
{ | ||
try { | ||
var invocableMirror = obj as IInvocableMethodOwnerMirror; | ||
if (invocableMirror != null) { | ||
var optionsToInvoke = options; | ||
if (obj is StructMirror) { | ||
optionsToInvoke |= InvokeOptions.ReturnOutThis; | ||
} | ||
handle = invocableMirror.BeginInvokeMethod (ctx.Thread, function, args, optionsToInvoke, null, null); | ||
get | ||
{ | ||
try { | ||
return function.DeclaringType.FullName + "." + function.Name; | ||
} | ||
catch (Exception e) { | ||
DebuggerLoggingService.LogError ("Exception during getting description of method", e); | ||
return "[Unknown method]"; | ||
} | ||
else | ||
throw new ArgumentException ("Soft debugger method calls cannot be invoked on objects of type " + obj.GetType ().Name); | ||
} catch (InvocationException ex) { | ||
ctx.Session.StackVersion++; | ||
exception = ex; | ||
} catch (Exception ex) { | ||
ctx.Session.StackVersion++; | ||
DebuggerLoggingService.LogError ("Error in soft debugger method call thread on " + GetInfo (), ex); | ||
exception = ex; | ||
} | ||
} | ||
|
||
public override void Abort () | ||
{ | ||
if (handle is IInvokeAsyncResult) { | ||
var info = GetInfo (); | ||
DebuggerLoggingService.LogMessage ("Aborting invocation of " + info); | ||
((IInvokeAsyncResult) handle).Abort (); | ||
// Don't wait for the abort to finish. The engine will do it. | ||
} else { | ||
throw new NotSupportedException (); | ||
} | ||
} | ||
|
||
public override void Shutdown () | ||
{ | ||
shutdownEvent.Set (); | ||
} | ||
|
||
void EndInvoke () | ||
protected override Task<OperationResult<Value>> InvokeAsyncImpl () | ||
{ | ||
try { | ||
result = ((IInvocableMethodOwnerMirror) obj).EndInvokeMethodWithResult (handle); | ||
} catch (InvocationException ex) { | ||
if (!Aborting && ex.Exception != null) { | ||
string ename = ctx.Adapter.GetValueTypeName (ctx, ex.Exception); | ||
var vref = ctx.Adapter.GetMember (ctx, null, ex.Exception, "Message"); | ||
|
||
exception = vref != null ? new Exception (ename + ": " + (string) vref.ObjectValue) : new Exception (ename); | ||
return; | ||
var optionsToInvoke = options; | ||
if (obj is StructMirror) { | ||
optionsToInvoke |= InvokeOptions.ReturnOutThis; | ||
} | ||
exception = ex; | ||
} catch (Exception ex) { | ||
DebuggerLoggingService.LogError ("Error in soft debugger method call thread on " + GetInfo (), ex); | ||
exception = ex; | ||
} finally { | ||
ctx.Session.StackVersion++; | ||
var tcs = new TaskCompletionSource<OperationResult<Value>> (); | ||
invokeAsyncResult = (IInvokeAsyncResult)obj.BeginInvokeMethod (ctx.Thread, function, args, optionsToInvoke, ar => { | ||
try { | ||
if (Token.IsCancellationRequested) { | ||
tcs.SetCanceled (); | ||
return; | ||
} | ||
var endInvokeResult = obj.EndInvokeMethodWithResult (ar); | ||
tcs.SetResult (new SoftOperationResult (endInvokeResult.Result, false, endInvokeResult.OutArgs)); | ||
} | ||
catch (InvocationException ex) { | ||
if (ex.Exception != null) { | ||
tcs.SetResult (new SoftOperationResult (ex.Exception, true, null)); | ||
} | ||
else { | ||
tcs.SetException (new EvaluatorException ("Target method has thrown an exception but the exception object is inaccessible")); | ||
} | ||
} | ||
catch (CommandException e) { | ||
if (e.ErrorCode == ErrorCode.INVOKE_ABORTED) { | ||
tcs.TrySetCanceled (); | ||
} | ||
else { | ||
tcs.SetException (new EvaluatorException (e.Message)); | ||
} | ||
} | ||
catch (Exception e) { | ||
if (e is ObjectCollectedException || | ||
e is InvalidStackFrameException || | ||
e is VMNotSuspendedException || | ||
e is NotSupportedException || | ||
e is AbsentInformationException || | ||
e is ArgumentException) { | ||
// user meaningfull evaluation exception -> wrap with EvaluatorException that will be properly shown in value viewer | ||
tcs.SetException (new EvaluatorException (e.Message)); | ||
} | ||
else { | ||
DebuggerLoggingService.LogError (string.Format ("Unexpected exception has thrown when ending invocation of {0}", GetInfo ()), e); | ||
tcs.SetException (e); | ||
} | ||
} | ||
finally { | ||
UpdateSessionState (); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. More logic that has been removed for no apparent good reason. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you mean logic of getting exception message so look at line 2230. SoftOperationResult wraps exception object to show it like ordinary object for user There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean GetInfo(), which is used to log the method and type name when there is an invocation error. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems that GetInfo used only for logging. Isnt't it enough to use Description for this? Now I log all this states using this property There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if it is enough or not, it needs to be checked. I just don't want code to be removed if there isn't a good reason. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The main reasons to remove are simplifying and avoiding duplications. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's bring it back then. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! |
||
} | ||
}, null); | ||
return tcs.Task; | ||
} catch (Exception e) { | ||
UpdateSessionState (); | ||
DebuggerLoggingService.LogError (string.Format ("Unexpected exception has thrown when invoking {0}", GetInfo ()), e); | ||
throw; | ||
} | ||
} | ||
|
||
string GetInfo () | ||
{ | ||
try { | ||
|
@@ -2250,41 +2261,26 @@ string GetInfo () | |
else if (obj is StructMirror) | ||
type = ((StructMirror)obj).Type; | ||
return string.Format ("method {0} on object {1}", | ||
function == null? "[null]" : function.FullName, | ||
type == null? "[null]" : type.FullName); | ||
function.FullName, | ||
type == null? "[null]" : type.FullName); | ||
} catch (Exception ex) { | ||
DebuggerLoggingService.LogError ("Error getting info for SDB MethodCall", ex); | ||
return ""; | ||
return "[Unknown method]"; | ||
} | ||
} | ||
|
||
public override bool WaitForCompleted (int timeout) | ||
void UpdateSessionState () | ||
{ | ||
if (handle == null) | ||
return true; | ||
int res = WaitHandle.WaitAny (new WaitHandle[] { handle.AsyncWaitHandle, shutdownEvent }, timeout); | ||
if (res == 0) { | ||
EndInvoke (); | ||
return true; | ||
} | ||
// Return true if shut down. | ||
return res == 1; | ||
} | ||
|
||
public Value ReturnValue { | ||
get { | ||
if (exception != null) | ||
throw new EvaluatorException (exception.Message); | ||
return result.Result; | ||
} | ||
ctx.Session.StackVersion++; | ||
} | ||
|
||
public Value[] OutArgs { | ||
get { | ||
if (exception != null) | ||
throw new EvaluatorException (exception.Message); | ||
return result.OutArgs; | ||
protected override void AbortImpl (int abortCallTimes) | ||
{ | ||
if (invokeAsyncResult == null) { | ||
DebuggerLoggingService.LogError ("invokeAsyncResult is null", new ArgumentNullException ("invokeAsyncResult")); | ||
return; | ||
} | ||
invokeAsyncResult.Abort (); | ||
} | ||
} | ||
} |
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.
Why is this logic gone?
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've replaced simple Exception message to full exception object. So now you can walk through it, but before you can see only exception message
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.
How is that object shown to the user?
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.
It's an ObjectValue with its children that has IsException flag. It's shown in watchpad as ordinary object