-
Notifications
You must be signed in to change notification settings - Fork 79
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 16 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 (); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,47 +1,143 @@ | ||
using System.Threading; | ||
using System; | ||
using System.Runtime.InteropServices; | ||
using System.Threading.Tasks; | ||
using Microsoft.Samples.Debugging.CorDebug; | ||
using Microsoft.Samples.Debugging.CorDebug.NativeApi; | ||
using Mono.Debugging.Client; | ||
using Mono.Debugging.Evaluation; | ||
|
||
namespace Mono.Debugging.Win32 | ||
{ | ||
class CorMethodCall: AsyncOperation | ||
class CorMethodCall: AsyncOperationBase<CorValue> | ||
{ | ||
public delegate void CallCallback ( ); | ||
public delegate string DescriptionCallback ( ); | ||
readonly CorEvaluationContext context; | ||
readonly CorFunction function; | ||
readonly CorType[] typeArgs; | ||
readonly CorValue[] args; | ||
|
||
public CallCallback OnInvoke; | ||
public CallCallback OnAbort; | ||
public DescriptionCallback OnGetDescription; | ||
readonly CorEval eval; | ||
|
||
public ManualResetEvent DoneEvent = new ManualResetEvent (false); | ||
|
||
public override string Description | ||
public CorMethodCall (CorEvaluationContext context, CorFunction function, CorType[] typeArgs, CorValue[] args) | ||
{ | ||
get { return OnGetDescription (); } | ||
this.context = context; | ||
this.function = function; | ||
this.typeArgs = typeArgs; | ||
this.args = args; | ||
eval = context.Eval; | ||
} | ||
|
||
public override void Invoke ( ) | ||
void ProcessOnEvalComplete (object sender, CorEvalEventArgs evalArgs) | ||
{ | ||
OnInvoke (); | ||
DoProcessEvalFinished (evalArgs, false); | ||
} | ||
|
||
public override void Abort ( ) | ||
void ProcessOnEvalException (object sender, CorEvalEventArgs evalArgs) | ||
{ | ||
OnAbort (); | ||
DoProcessEvalFinished (evalArgs, true); | ||
} | ||
|
||
public override void Shutdown ( ) | ||
void DoProcessEvalFinished (CorEvalEventArgs evalArgs, bool isException) | ||
{ | ||
try { | ||
Abort (); | ||
if (evalArgs.Eval != eval) | ||
return; | ||
context.Session.OnEndEvaluating (); | ||
evalArgs.Continue = false; | ||
if (Token.IsCancellationRequested) { | ||
DebuggerLoggingService.LogMessage ("EvalFinished() but evaluation was cancelled"); | ||
tcs.TrySetCanceled (); | ||
} | ||
catch { | ||
else { | ||
DebuggerLoggingService.LogMessage ("EvalFinished(). Setting the result"); | ||
tcs.TrySetResult(new OperationResult<CorValue> (evalArgs.Eval.Result, isException)); | ||
} | ||
DoneEvent.Set (); | ||
} | ||
|
||
public override bool WaitForCompleted (int timeout) | ||
void SubscribeOnEvals () | ||
{ | ||
context.Session.Process.OnEvalComplete += ProcessOnEvalComplete; | ||
context.Session.Process.OnEvalException += ProcessOnEvalException; | ||
} | ||
|
||
void UnSubcribeOnEvals () | ||
{ | ||
return DoneEvent.WaitOne (timeout, false); | ||
context.Session.Process.OnEvalComplete -= ProcessOnEvalComplete; | ||
context.Session.Process.OnEvalException -= ProcessOnEvalException; | ||
} | ||
|
||
public override string Description | ||
{ | ||
get | ||
{ | ||
var met = function.GetMethodInfo (context.Session); | ||
if (met == null) | ||
return "[Unknown method]"; | ||
if (met.DeclaringType == null) | ||
return met.Name; | ||
return met.DeclaringType.FullName + "." + met.Name; | ||
} | ||
} | ||
|
||
readonly TaskCompletionSource<OperationResult<CorValue>> tcs = new TaskCompletionSource<OperationResult<CorValue>> (); | ||
|
||
protected override Task<OperationResult<CorValue>> InvokeAsyncImpl () | ||
{ | ||
SubscribeOnEvals (); | ||
|
||
if (function.GetMethodInfo (context.Session).Name == ".ctor") | ||
eval.NewParameterizedObject (function, typeArgs, args); | ||
else | ||
eval.CallParameterizedFunction (function, typeArgs, args); | ||
context.Session.Process.SetAllThreadsDebugState (CorDebugThreadState.THREAD_SUSPEND, context.Thread); | ||
context.Session.ClearEvalStatus (); | ||
context.Session.OnStartEvaluating (); | ||
context.Session.Process.Continue (false); | ||
Task = tcs.Task; | ||
// Don't pass token here, because it causes immediately task cancellation which must be performed by debugger event or real timeout | ||
return Task.ContinueWith (task => { | ||
UnSubcribeOnEvals (); | ||
return task.Result; | ||
}); | ||
} | ||
|
||
|
||
protected override void AbortImpl (int abortCallTimes) | ||
{ | ||
try { | ||
if (abortCallTimes < 10) { | ||
DebuggerLoggingService.LogMessage ("Calling Abort() for {0} time", abortCallTimes); | ||
eval.Abort (); | ||
} | ||
else { | ||
if (abortCallTimes == 20) { | ||
// if Abort() and RudeAbort() didn't bring any result let's try to resume all the threads to free possible deadlocks in target process | ||
// maybe this can help to abort hanging evaluations | ||
DebuggerLoggingService.LogMessage ("RudeAbort() didn't stop eval after {0} times", abortCallTimes - 1); | ||
DebuggerLoggingService.LogMessage ("Calling Stop()"); | ||
context.Session.Process.Stop (0); | ||
DebuggerLoggingService.LogMessage ("Calling SetAllThreadsDebugState(THREAD_RUN)"); | ||
context.Session.Process.SetAllThreadsDebugState (CorDebugThreadState.THREAD_RUN, null); | ||
DebuggerLoggingService.LogMessage ("Calling Continue()"); | ||
context.Session.Process.Continue (false); | ||
} | ||
DebuggerLoggingService.LogMessage ("Calling RudeAbort() for {0} time", abortCallTimes); | ||
eval.RudeAbort(); | ||
} | ||
|
||
} catch (COMException e) { | ||
var hResult = e.ToHResult<HResult> (); | ||
switch (hResult) { | ||
case HResult.CORDBG_E_PROCESS_TERMINATED: | ||
DebuggerLoggingService.LogMessage ("Process was terminated. Set cancelled for eval"); | ||
tcs.TrySetCanceled (); | ||
return; | ||
case HResult.CORDBG_E_OBJECT_NEUTERED: | ||
DebuggerLoggingService.LogMessage ("Eval object was neutered. Set cancelled for eval"); | ||
tcs.TrySetCanceled (); | ||
return; | ||
} | ||
tcs.SetException (e); | ||
throw; | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -41,7 +41,7 @@ namespace Mono.Debugging.Evaluation | |||
/// will then be made asynchronous and the Run method will immediately return an ObjectValue | ||||
/// with the Evaluating state. | ||||
/// </summary> | ||||
public class AsyncEvaluationTracker: RemoteFrameObject, IObjectValueUpdater, IDisposable | ||||
public class AsyncEvaluationTracker: IObjectValueUpdater, IDisposable | ||||
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. Why this change? 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. 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 used by some dynamic way I can revert this change 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 important bit is that RemoteFrameObject is a MarshalByRefObject, and AsyncEvaluationTracker needs to be remotable. 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. Can you please explain how is this working? where to look at the remoting? 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 library is designed so that the debugging engine can run in a remote process, which communicates with the client using remoting. That's really up to the debugger engine implementation. In such scenario, the AsyncEvaluationTracker could be executing in the remote process and generating ObjectValue instances that are serialized into the client process and which may need to hold a reference to that remote object (
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. Please restore the RemoteFrameObject base class. |
||||
{ | ||||
Dictionary<string, UpdateCallback> asyncCallbacks = new Dictionary<string, UpdateCallback> (); | ||||
Dictionary<string, ObjectValue> asyncResults = new Dictionary<string, ObjectValue> (); | ||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
using System; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Mono.Debugging.Client; | ||
|
||
namespace Mono.Debugging.Evaluation | ||
{ | ||
public class OperationResult<TValue> | ||
{ | ||
public TValue Result { get; private set; } | ||
public bool ResultIsException { get; private set; } | ||
|
||
public OperationResult (TValue result, bool resultIsException) | ||
{ | ||
Result = result; | ||
ResultIsException = resultIsException; | ||
} | ||
} | ||
|
||
public static class OperationResultEx | ||
{ | ||
public static OperationResult<TValue> ThrowIfException<TValue> (this OperationResult<TValue> result, EvaluationContext ctx) | ||
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. Why not just make it a member of OperationResult? 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 guess this approach makes the code more simple. OperationResult is just a data class without any internal logic 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. That looks unnecessary to me. In any case, up to you. 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. In this case ThrowIfException is more helper method rather than required to be in the class. It's IMHO |
||
{ | ||
if (!result.ResultIsException) | ||
return result; | ||
var exceptionTypeName = ctx.Adapter.GetValueTypeName (ctx, result.Result); | ||
throw new EvaluatorExceptionThrownException (result.Result, exceptionTypeName); | ||
} | ||
} | ||
|
||
public interface IAsyncOperationBase | ||
{ | ||
Task RawTask { get; } | ||
string Description { get; } | ||
void Abort (); | ||
} | ||
|
||
public abstract class AsyncOperationBase<TValue> : IAsyncOperationBase | ||
{ | ||
public Task<OperationResult<TValue>> Task { get; protected set; } | ||
|
||
public Task RawTask | ||
{ | ||
get | ||
{ | ||
return Task; | ||
} | ||
} | ||
|
||
public abstract string Description { get; } | ||
|
||
int abortCalls = 0; | ||
|
||
readonly CancellationTokenSource tokenSource = new CancellationTokenSource (); | ||
|
||
/// <summary> | ||
/// When evaluation is aborted and debugger callback is invoked the implementation has to check | ||
/// for Token.IsCancellationRequested and call Task.SetCancelled() instead of setting the result | ||
/// </summary> | ||
protected CancellationToken Token { get { return tokenSource.Token; } } | ||
|
||
public void Abort () | ||
{ | ||
try { | ||
tokenSource.Cancel(); | ||
AbortImpl (Interlocked.Increment (ref abortCalls) - 1); | ||
} | ||
catch (OperationCanceledException) { | ||
// if CancelImpl throw OCE we shouldn't mute it | ||
throw; | ||
} | ||
catch (Exception e) { | ||
DebuggerLoggingService.LogMessage ("Exception in CancelImpl(): {0}", e.Message); | ||
} | ||
} | ||
|
||
public Task<OperationResult<TValue>> InvokeAsync () | ||
{ | ||
if (Task != null) throw new Exception("Task must be null"); | ||
Task = InvokeAsyncImpl (); | ||
return Task; | ||
} | ||
|
||
protected abstract Task<OperationResult<TValue>> InvokeAsyncImpl (); | ||
|
||
/// <summary> | ||
/// The implementation has to tell the debugger to abort the evaluation. This method must bot block. | ||
/// </summary> | ||
/// <param name="abortCallTimes">indicates how many times this method has been already called for this evaluation. | ||
/// E.g. the implementation can perform some 'rude abort' after several previous ordinary 'aborts' were failed. For the first call this parameter == 0</param> | ||
protected abstract void AbortImpl (int abortCallTimes); | ||
|
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,215 +27,163 @@ | |
|
||
using System; | ||
using System.Collections.Generic; | ||
using ST = System.Threading; | ||
using System.Linq; | ||
using System.Threading.Tasks; | ||
using Mono.Debugging.Client; | ||
|
||
namespace Mono.Debugging.Evaluation | ||
{ | ||
public class AsyncOperationManager: IDisposable | ||
public class AsyncOperationManager : IDisposable | ||
{ | ||
List<AsyncOperation> operationsToCancel = new List<AsyncOperation> (); | ||
internal bool Disposing; | ||
readonly HashSet<IAsyncOperationBase> currentOperations = new HashSet<IAsyncOperationBase> (); | ||
bool disposed = false; | ||
const int ShortCancelTimeout = 100; | ||
|
||
public void Invoke (AsyncOperation methodCall, int timeout) | ||
static bool IsOperationCancelledException (Exception e, int depth = 4) | ||
{ | ||
methodCall.Aborted = false; | ||
methodCall.Manager = this; | ||
if (e is OperationCanceledException) | ||
return true; | ||
var aggregateException = e as AggregateException; | ||
|
||
lock (operationsToCancel) { | ||
operationsToCancel.Add (methodCall); | ||
methodCall.Invoke (); | ||
if (depth > 0 && aggregateException != null) { | ||
foreach (var innerException in aggregateException.InnerExceptions) { | ||
if (IsOperationCancelledException (innerException, depth - 1)) | ||
return true; | ||
} | ||
} | ||
return false; | ||
} | ||
|
||
public OperationResult<TValue> Invoke<TValue> (AsyncOperationBase<TValue> mc, int timeout) | ||
{ | ||
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. Why remove this check? 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. there is no Aborted property on AsyncOperation now, Cancellation is controlled by Task API 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. Ok |
||
if (timeout <= 0) | ||
throw new ArgumentOutOfRangeException("timeout", timeout, "timeout must be greater than 0"); | ||
|
||
Task<OperationResult<TValue>> task; | ||
var description = mc.Description; | ||
lock (currentOperations) { | ||
if (disposed) | ||
throw new ObjectDisposedException ("Already disposed"); | ||
DebuggerLoggingService.LogMessage (string.Format("Starting invoke for {0}", description)); | ||
task = mc.InvokeAsync (); | ||
currentOperations.Add (mc); | ||
} | ||
|
||
if (timeout > 0) { | ||
if (!methodCall.WaitForCompleted (timeout)) { | ||
bool wasAborted = methodCall.Aborted; | ||
methodCall.InternalAbort (); | ||
lock (operationsToCancel) { | ||
operationsToCancel.Remove (methodCall); | ||
ST.Monitor.PulseAll (operationsToCancel); | ||
bool cancelledAfterTimeout = false; | ||
try { | ||
if (task.Wait (timeout)) { | ||
DebuggerLoggingService.LogMessage (string.Format ("Invoke {0} succeeded in {1} ms", description, timeout)); | ||
return task.Result; | ||
} | ||
DebuggerLoggingService.LogMessage (string.Format ("Invoke {0} timed out after {1} ms. Cancelling.", description, timeout)); | ||
mc.Abort (); | ||
try { | ||
WaitAfterCancel (mc); | ||
} | ||
catch (Exception e) { | ||
if (IsOperationCancelledException (e)) { | ||
DebuggerLoggingService.LogMessage (string.Format ("Invoke {0} was cancelled after timeout", description)); | ||
cancelledAfterTimeout = true; | ||
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. All the Shutdown logic is gone, why? 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. Shutdown had nearly the same logic as the Cancel. I've unified them into Cancel. 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. Nearly the same logic, but not the same. Let's say an invocation is made. While the caller is waiting for the invocation to end, the manager is disposed, so the call is canceled. But if the call can't really be cancelled, the caller will wait forever. 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. Why do you think so? After Dispose is called the Invoke method on another thread will throw EvaluationAbortedException 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 the .Abort() call on the invocation fails, or can't be completed (it can happen, that's why there is all that abort retry and debugger busy mode infrastructure), then the invocation will never end, and EvaluationAbortedException will never be thrown. 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. Fixed. See below |
||
} | ||
if (wasAborted) | ||
throw new EvaluatorAbortedException (); | ||
else | ||
throw new TimeOutException (); | ||
throw; | ||
} | ||
DebuggerLoggingService.LogMessage (string.Format ("{0} cancelling timed out", description)); | ||
throw new TimeOutException (); | ||
} | ||
else { | ||
methodCall.WaitForCompleted (System.Threading.Timeout.Infinite); | ||
} | ||
|
||
lock (operationsToCancel) { | ||
operationsToCancel.Remove (methodCall); | ||
ST.Monitor.PulseAll (operationsToCancel); | ||
if (methodCall.Aborted) { | ||
catch (Exception e) { | ||
if (IsOperationCancelledException (e)) { | ||
if (cancelledAfterTimeout) | ||
throw new TimeOutException (); | ||
DebuggerLoggingService.LogMessage (string.Format ("Invoke {0} was cancelled outside before timeout", description)); | ||
throw new EvaluatorAbortedException (); | ||
} | ||
throw; | ||
} | ||
finally { | ||
lock (currentOperations) { | ||
currentOperations.Remove (mc); | ||
} | ||
} | ||
} | ||
|
||
|
||
if (!string.IsNullOrEmpty (methodCall.ExceptionMessage)) { | ||
throw new Exception (methodCall.ExceptionMessage); | ||
public event EventHandler<BusyStateEventArgs> BusyStateChanged = delegate { }; | ||
|
||
void ChangeBusyState (bool busy, string description) | ||
{ | ||
try { | ||
BusyStateChanged (this, new BusyStateEventArgs {IsBusy = busy, Description = description}); | ||
} | ||
catch (Exception e) { | ||
DebuggerLoggingService.LogError ("Exception during ChangeBusyState", e); | ||
} | ||
} | ||
public void Dispose () | ||
|
||
void WaitAfterCancel (IAsyncOperationBase op) | ||
{ | ||
Disposing = true; | ||
lock (operationsToCancel) { | ||
foreach (AsyncOperation op in operationsToCancel) { | ||
op.InternalShutdown (); | ||
var desc = op.Description; | ||
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. It doesn't take into account that the method may already be being aborted, like the old implementation did. |
||
DebuggerLoggingService.LogMessage (string.Format ("Waiting for cancel of invoke {0}", desc)); | ||
if (!op.RawTask.Wait (ShortCancelTimeout)) { | ||
try { | ||
ChangeBusyState (true, desc); | ||
while (true) { | ||
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. This loop should gracefully exit if AsyncOperationManager is disposed. 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. Logic for switching to busy state is now different. Please use old logic. 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 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.
Can you please explain more detailed?
Seems something like that was before and implemented on magic counters. And the code was very confusing |
||
op.Abort (); | ||
if (op.RawTask.Wait (ShortCancelTimeout)) | ||
break; | ||
} | ||
} | ||
finally { | ||
ChangeBusyState (false, desc); | ||
} | ||
operationsToCancel.Clear (); | ||
} | ||
} | ||
|
||
public void AbortAll () | ||
{ | ||
lock (operationsToCancel) { | ||
foreach (AsyncOperation op in operationsToCancel) | ||
op.InternalAbort (); | ||
DebuggerLoggingService.LogMessage ("Aborting all the current invocations"); | ||
List<IAsyncOperationBase> copy; | ||
lock (currentOperations) { | ||
if (disposed) throw new ObjectDisposedException ("Already disposed"); | ||
copy = currentOperations.ToList (); | ||
currentOperations.Clear (); | ||
} | ||
|
||
CancelOperations (copy, true); | ||
} | ||
|
||
public void EnterBusyState (AsyncOperation oper) | ||
{ | ||
BusyStateEventArgs args = new BusyStateEventArgs (); | ||
args.IsBusy = true; | ||
args.Description = oper.Description; | ||
if (BusyStateChanged != null) | ||
BusyStateChanged (this, args); | ||
} | ||
|
||
public void LeaveBusyState (AsyncOperation oper) | ||
{ | ||
BusyStateEventArgs args = new BusyStateEventArgs (); | ||
args.IsBusy = false; | ||
args.Description = oper.Description; | ||
if (BusyStateChanged != null) | ||
BusyStateChanged (this, args); | ||
} | ||
|
||
public event EventHandler<BusyStateEventArgs> BusyStateChanged; | ||
} | ||
|
||
public abstract class AsyncOperation | ||
{ | ||
internal bool Aborted; | ||
internal AsyncOperationManager Manager; | ||
|
||
public bool Aborting { get; internal set; } | ||
|
||
internal void InternalAbort () | ||
void CancelOperations (List<IAsyncOperationBase> operations, bool wait) | ||
{ | ||
ST.Monitor.Enter (this); | ||
if (Aborted) { | ||
ST.Monitor.Exit (this); | ||
return; | ||
} | ||
|
||
if (Aborting) { | ||
// Somebody else is aborting this. Just wait for it to finish. | ||
ST.Monitor.Exit (this); | ||
WaitForCompleted (ST.Timeout.Infinite); | ||
return; | ||
} | ||
|
||
Aborting = true; | ||
|
||
int abortState = 0; | ||
int abortRetryWait = 100; | ||
bool abortRequested = false; | ||
|
||
do { | ||
if (abortState > 0) | ||
ST.Monitor.Enter (this); | ||
|
||
foreach (var operation in operations) { | ||
var taskDescription = operation.Description; | ||
try { | ||
if (!Aborted && !abortRequested) { | ||
// The Abort() call doesn't block. WaitForCompleted is used below to wait for the abort to succeed | ||
Abort (); | ||
abortRequested = true; | ||
} | ||
// Short wait for the Abort to finish. If this wait is not enough, it will wait again in the next loop | ||
if (WaitForCompleted (100)) { | ||
ST.Monitor.Exit (this); | ||
break; | ||
operation.Abort (); | ||
if (wait) { | ||
WaitAfterCancel (operation); | ||
} | ||
} catch { | ||
// If abort fails, try again after a short wait | ||
} | ||
abortState++; | ||
if (abortState == 6) { | ||
// Several abort calls have failed. Inform the user that the debugger is busy | ||
abortRetryWait = 500; | ||
try { | ||
Manager.EnterBusyState (this); | ||
} catch (Exception ex) { | ||
Console.WriteLine (ex); | ||
catch (Exception e) { | ||
if (IsOperationCancelledException (e)) { | ||
DebuggerLoggingService.LogMessage (string.Format ("Invocation of {0} cancelled in CancelOperations()", taskDescription)); | ||
} | ||
else { | ||
DebuggerLoggingService.LogError (string.Format ("Invocation of {0} thrown an exception in CancelOperations()", taskDescription), e); | ||
} | ||
} | ||
ST.Monitor.Exit (this); | ||
} while (!Aborted && !WaitForCompleted (abortRetryWait) && !Manager.Disposing); | ||
|
||
if (Manager.Disposing) { | ||
InternalShutdown (); | ||
} | ||
else { | ||
lock (this) { | ||
Aborted = true; | ||
if (abortState >= 6) | ||
Manager.LeaveBusyState (this); | ||
} | ||
} | ||
} | ||
|
||
internal void InternalShutdown () | ||
|
||
|
||
public void Dispose () | ||
{ | ||
lock (this) { | ||
if (Aborted) | ||
return; | ||
try { | ||
Aborted = true; | ||
Shutdown (); | ||
} catch { | ||
// Ignore | ||
} | ||
List<IAsyncOperationBase> copy; | ||
lock (currentOperations) { | ||
if (disposed) throw new ObjectDisposedException ("Already disposed"); | ||
disposed = true; | ||
copy = currentOperations.ToList (); | ||
currentOperations.Clear (); | ||
} | ||
// don't wait on dispose | ||
CancelOperations (copy, wait: false); | ||
} | ||
|
||
/// <summary> | ||
/// Message of the exception, if the execution failed. | ||
/// </summary> | ||
public string ExceptionMessage { get; set; } | ||
|
||
/// <summary> | ||
/// Returns a short description of the operation, to be shown in the Debugger Busy Dialog | ||
/// when it blocks the execution of the debugger. | ||
/// </summary> | ||
public abstract string Description { get; } | ||
|
||
/// <summary> | ||
/// Called to invoke the operation. The execution must be asynchronous (it must return immediatelly). | ||
/// </summary> | ||
public abstract void Invoke ( ); | ||
|
||
/// <summary> | ||
/// Called to abort the execution of the operation. It has to throw an exception | ||
/// if the operation can't be aborted. This operation must not block. The engine | ||
/// will wait for the operation to be aborted by calling WaitForCompleted. | ||
/// </summary> | ||
public abstract void Abort (); | ||
|
||
/// <summary> | ||
/// Waits until the operation has been completed or aborted. | ||
/// </summary> | ||
public abstract bool WaitForCompleted (int timeout); | ||
|
||
/// <summary> | ||
/// Called when the debugging session has been disposed. | ||
/// I must cause any call to WaitForCompleted to exit, even if the operation | ||
/// has not been completed or can't be aborted. | ||
/// </summary> | ||
public abstract void Shutdown (); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
namespace Mono.Debugging.Evaluation | ||
{ | ||
public interface IAsyncOperation | ||
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. This seems to be unused. 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. That's right! Thanks 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. Still unused... |
||
{ | ||
/// <summary> | ||
/// Called to invoke the operation. The execution must be asynchronous (it must return immediatelly). | ||
/// </summary> | ||
void BeginInvoke (); | ||
|
||
/// <summary> | ||
/// Called to abort the execution of the operation. It has to throw an exception | ||
/// if the operation can't be aborted. This operation must not block. The engine | ||
/// will wait for the operation to be aborted by calling WaitForCompleted. | ||
/// </summary> | ||
void Abort (); | ||
|
||
/// <summary> | ||
/// Waits until the operation has been completed or aborted. | ||
/// </summary> | ||
bool WaitForCompleted (int timeout); | ||
} | ||
} |
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