0

I am trying to enable a Kill option for a case that runs asynchronously from the calling thread, as a Process. My example looks like @svick's answer here. I am trying to implement the recommendation by @svick in this post. But when I click on Kill in the UI, it appears to do nothing (i.e., the process simply runs to completion as usual).

As per @TimCopenhaver's response, this is expected. But if I commented that out, it still doesn't do anything, this time because the CancellationTokenSource object cts is null, which is unexpected since I instantiate it in the Dispatch method of the CaseDispatcher class before trying to kill it. Here is my code snippet:

UI class:

private void OnKillCase(object sender, EventArgs args)
{
    foreach (var case in Cases)
    {
        Args caseArgs = CaseAPI.GetCaseArguments(case);
        CaseDispatcher dispatcher = CaseAPI.GetCaseDispatcher(case);
        dispatcher.Kill();
        CaseAPI.Dispose(caseArgs); 
    }
}

CaseDispatcher class:

    private Task<bool> task;
    private CancellationTokenSource cts;
    public override bool IsRunning
    {
        get { return task != null && task.Status == TaskStatus.Running; }
    }
    public override void Kill()
    {
        //if (!IsRunning)
        //{
        //    return;
        //}
        if (cts != null)
        {
            cts.Cancel();
        }
    }
    public override async Task<bool> Dispatch()
    {
        cts = new CancellationTokenSource();
        task = CaseAPI.Dispatch(Arguments, cts.Token);
        return await task;
    }

CaseAPI class:

public static async Task<bool> Dispatch(CaseArgs args, CancellationToken ctoken)
{
    bool ok = true;
    BatchEngine engine = new BatchEngine()
        {
            Spec = somespec,
            CaseName = args.CaseName,
            CaseDirectory = args.CaseDirectory
        };
    ok &= await engine.ExecuteAsync(ctoken);
    return ok;
}

BatchEngine class (here is where I invoke the CancellationToken Register method, but not sure exactly where to position it, assuming it matters):

public virtual Task<bool> ExecuteAsync(CancellationToken ctoken)
{
    var tcs = new TaskCompletionSource<bool>();
    string exe = Spec.GetExecutablePath();
    string args = string.Format("--input={0} {1}", Input, ConfigFile);

    try
    {
        var process = new Process
        {
            EnableRaisingEvents = true,
            StartInfo =
            {
                UseShellExecute = false,
                FileName = exe,
                Arguments = args,
                CreateNoWindow = true,
                RedirectStandardOutput = true,
                RedirectStandardError = true,
                WorkingDirectory = CaseDirectory
            }
        };
        ctoken.Register(() =>
            { 
                process.Kill();
                process.Dispose();
                tcs.SetResult(false);
            });
        process.Exited += (sender, arguments) =>
        {
            if (process.ExitCode != 0)
            {
                string errorMessage = process.StandardError.ReadToEnd();
                tcs.SetResult(false);
                tcs.SetException(new InvalidOperationException("The batch process did not exit correctly. Error message: " + errorMessage));
            }
            else
            {
                File.WriteAllText(LogFile, process.StandardOutput.ReadToEnd());
                tcs.SetResult(true);
            }
            process.Dispose();
        };
        process.Start();
    }
    catch (Exception e)
    {
        Logger.InfoOutputWindow(e.Message);
        tcs.SetResult(false);
        return tcs.Task;
    }
    return tcs.Task;
}

Thanks for your interest and appreciate any ideas on this.

Community
  • 1
  • 1
squashed.bugaboo
  • 1,338
  • 2
  • 20
  • 36
  • `ctoken.Register` returns a object that should be disposed of in the `Exited` event handler. – Scott Chamberlain Jun 20 '16 at 21:23
  • 2
    This should basically work (besides a few race conditions likely not causing your immediate issue). Place breakpoints on cts.Cancel and process.Kill to see where it goes wrong. Bisect. – usr Jun 20 '16 at 21:25
  • Can you show the code of your IsRunning property for CaseDispatcher? Where does that get set? – Tim Copenhaver Jun 20 '16 at 21:38
  • @TimCopenhaver: I included the accessor that just now. Good point, looks like I am not even entering the Kill. Perhaps because by that point the private task object (turns out to be null) is not yet assigned since the batch process is obviously not finished. I'll call cts.Cancel() regardless whenever Kill is invoked by user and see if it gets me further. – squashed.bugaboo Jun 20 '16 at 21:52
  • @usr: You're right, I tested it works OK now my issues were mostly peripheral as pointed out by Tim Copenhaver – squashed.bugaboo Jun 21 '16 at 22:05

1 Answers1

1

I believe the IsRunning property is the problem. Because the TaskCompletionSource doesn't really know you started the external process, it is stuck in the WaitingForActivation state. Here's a simplified example to demonstrate:

var tsc = new TaskCompletionSource<int>();

Task.Factory.StartNew(() =>
{
    Thread.Sleep(10000);
    tsc.SetResult(10);
});

var tmp = tsc.Task;

TaskStatus status = tmp.Status;
while (status != TaskStatus.RanToCompletion)
{
    status = tmp.Status;
    Thread.Sleep(1000);
    Console.WriteLine(status);
}

Notice it will keep saying WaitingForActivation up until it switches to RanToCompletion. See this answer for more discussion on this. In short, if the task is created by a TaskCompletionSource, it will never enter the running state. You will have to manage the IsRunning property yourself.

Community
  • 1
  • 1
Tim Copenhaver
  • 3,282
  • 13
  • 18
  • From what I learnt so far, for batch processes which use the Process, we have to use TaskCompletionSource to run asynchronously from the calling thread, correct? This means there is no way around this problem, if we want to kill such processes. Also, when I debug this I notice the cts is also null in the Kill method, and so again, it doesn't enter, even though cts is instantiated, this was unexpected to me. Any ideas on that? – squashed.bugaboo Jun 20 '16 at 22:26
  • 1
    You can still cancel, but there's no workaround for being unable to check the status of the running task. For the second issue with cts being null, I suspect a bug in GetCaseDispatcher. If the CaseDispatcher instance isn't cached correctly you may be getting a new instance back which doesn't have the properties set. – Tim Copenhaver Jun 20 '16 at 22:53
  • I was suspecting the same and from reviewing the code, looks like you're right again. This framework I inherited appears very buggy. Will try to fix and report back. – squashed.bugaboo Jun 21 '16 at 13:27