Laden...

Task basierte Kommandozeilen-Applikaton "graceful" beenden

Erstellt von sandreas vor einem Jahr Letzter Beitrag vor einem Jahr 691 Views
S
sandreas Themenstarter:in
29 Beiträge seit 2022
vor einem Jahr
Task basierte Kommandozeilen-Applikaton "graceful" beenden

Hallo zusammen,

ich habe kürzlich hier meinen Audio-Tagger tone (https://github.com/sandreas/tone) vorgestellt und nun ein (für mich) interessantes Problem. Und zwar hat mir jemand gemeldet, dass das Drücken von STRG+C inmitten der Veränderung der Metadaten dazu führt, dass Dateien "kaputtgeschrieben" werden, weil die Anwendung ohne Vorwarnung terminiert.

Nun habe ich mich etwas eingelesen und einen Vorschlag eines andren Entwicklers für die Kommandozeilen-Bibliothek spectre.console dazu mehr oder minder unverändert übernommen (siehe hier https://github.com/spectreconsole/spectre.console/issues/701#issuecomment-1081834778):


internal class ConsoleAppCancellationTokenSource
{
    private readonly CancellationTokenSource _cts = new();

    public CancellationToken Token => _cts.Token;

    public ConsoleAppCancellationTokenSource()
    {
        Console.CancelKeyPress += OnCancelKeyPress;
        AppDomain.CurrentDomain.ProcessExit += OnProcessExit;

        using var _ = _cts.Token.Register(() =>
        {
            AppDomain.CurrentDomain.ProcessExit -= OnProcessExit;
            Console.CancelKeyPress -= OnCancelKeyPress;
        });
    }

    private void OnCancelKeyPress(object? sender, ConsoleCancelEventArgs e)
    {
        // NOTE: cancel event, don't terminate the process
        e.Cancel = true;

        _cts.Cancel();
    }

    private void OnProcessExit(object? sender, EventArgs e)
    {
        if (_cts.IsCancellationRequested)
        {
            // NOTE: SIGINT (cancel key was pressed, this shouldn't ever actually hit however, as we remove the event handler upon cancellation of the `cancellationSource`)
            return;
        }

        _cts.Cancel();
    }
}

Dieser sorgt nun dafür, dass ich beim Drücken von STRG+C oder beim einem Prozessignal (z.B. mit kill und Linux) einen CancellationToken abbreche, anstatt die Anwendung hart zu beenden (zumindest, wenn ich es richtig verstanden habe).

Nun muss ich diesen CancellationToken ja noch an die entsprechende Task-Verwaltung-Einheit übergeben. Das tue ich hier:

https://github.com/sandreas/tone/blob/main/tone/Commands/TagCommand.cs

mit


            var tasks = packages.Select(p => Task.Run(async () =>
            {
                foreach (var file in p.Files)
                {
                     // Datei laden und Tags verändern
                     // ...
                     // Änderungen speichern
                     track.Save()
                }
            }, cancellation)).ToArray();

            await Task.Run(() => Task.WaitAll(tasks), cancellation);

Wie zu sehen ist, teile ich die Dateien zunächst in "Datei-Pakete" auf, die ich dann verarbeite. Jedes dieser Pakete wird als async Task verpackt und über Task.Run mit dem CancellationToken wird die Verarbeitung gestartet. Nun möchte ich bei einem Cancel natürlich nicht warten, bis ein solches Paket komplett verarbeitet ist, da es aus vielen Dateien bestehen kann, sondern das Programm möglichst direkt beenden (falls die Bearbeitung des Pakets mit vielen Dateien sehr lange dauert).

Soweit ich verstanden habe, würde das CancelationTokenSource.Cancel() nun dazu führen, dass der angefangene async Task noch zu Ende läuft und das Programm dann terminiert. Demnach müsste ich folgendes tun, um "direkt" zu Beenden (foreach muss vorzeitig beendet werden):


            var tasks = packages.Select(p => Task.Run(async () =>
            {
                foreach (var file in p.Files)
                {

                     // NEUER CODE
                     if(cancellation.IsCancellationRequested) {
                         break;
                     }
                     // NEUER CODE ENDE

                     // Datei laden und Tags verändern
                     // ...
                     // Änderungen speichern
                     track.Save()
                }
            }, cancellation)).ToArray();

            await Task.Run(() => Task.WaitAll(tasks), cancellation);

Wäre das so richtig oder habe ich etwas übersehen / nicht verstanden?

16.806 Beiträge seit 2008
vor einem Jahr

var tasks = packages.Select(p => Task.Run(async () =>
{
    foreach (var file in p.Files)
    {
         // Datei laden und Tags verändern
         // ...
         // Änderungen speichern
         track.Save()
    }
}, cancellation)).ToArray();

Ist kein schlauer Code. Besser wäre es das gesamte Select in <einen> Task auszulagern und nicht dutzende zu erzeugen. Ist im Endeffekt so ein Anti-Pattern bzw. Pitfall bei der Task-Kaskadierung.
Dann lässt sich der Cancellation Token auch viel einfacher anwenden.

Schlau ist der Code übrigens auch nicht, weil sich parallele Zugriffe auf das Dateisystem in den aller meisten Fällen negativ auswirken (es wird langsamer).
Wo es hilft ist, wenn die Dateizugriffe über das Netzwerk erfolgen. Auch SSDs sind bei sequential read um Längen schneller als bei parallelen Aktionen.

Ansonsten: Ja, prinzipiell verstanden.
Das CancellationToken nimmt man für das bewusste, passive Abbrechen, sodass eine Anwendung sauber sich beenden kann.
Das ist auch eine der wichtigen Vorteile von Tasks gegenüber Threads, weil man letztere nur abschießen aber nicht abbrechen kann.

S
sandreas Themenstarter:in
29 Beiträge seit 2022
vor einem Jahr

Vielen Dank für die hilfreiche Antwort. Ja, das mit dem Select steht schon recht weit oben auf meiner Liste 🙂 Der Code ist echt Mist so. Ich hatte mal geplant, den Grad Parallelisierung einstellbar zu machen (z.B. mit --jobs=4 oder sowas) um Remote-Dateisysteme schneller zu scannen, aber ich sollte wohl eher YAGNI verfolgen und es sequenziell machen.

Aber deine Bemerkung

Dann lässt sich der Cancellation Token auch viel einfacher anwenden.

hab ich noch nicht richtig verstanden... ich denke, das klügste wäre für den Anfang, statt eines Select mit Tasks einfach ein foreach zu machen, also:


    public override async Task<int> ExecuteAsync(CommandContext context, TagCommandSettings settings,
        CancellationToken cancellation)
    {
        var showDryRunMessage = false;
        foreach (var p in packages)
        {
            if (await ProcessAudioBookPackage(p, settings, cancellation))
            {
                showDryRunMessage = true;
            }
        }
    }
    
    private async Task<bool> ProcessAudioBookPackage(AudioBookPackage p, TagCommandSettings settings,
        CancellationToken cancellation)
    {
        var showDryRunMessage = false;
        foreach (var file in p.Files)
        {
            if (cancellation.IsCancellationRequested)
            {
                break;
            }

            var track = new MetadataTrack(file)
            {
                BasePath = p.BaseDirectory?.FullName
            };
            var status = await _tagger.UpdateAsync(track);

            if (!status)
            {
                _console.Error.WriteLine($"Could not update tags for file {file}: {status.Error}");
                continue;
            }

            var currentMetadata = new MetadataTrack(file);
            var diffListing = track.Diff(currentMetadata);
            if (diffListing.Count == 0)
            {
                if (settings.DryRun || !settings.Force)
                {
                    _console.Write(new Rule($"[green]unchanged: {Markup.Escape(track.Path ?? "")}[/]")
                        .LeftAligned());
                    continue;
                }

                var path = Markup.Escape(track.Path ?? "");
                var message = !track.Save()
                    ? $"[red]Force update failed: {path}[/]"
                    : $"[green]Forced update: {path}[/]";
                _console.Write(new Rule(message)
                    .LeftAligned());
            }
            else
            {
                showDryRunMessage = settings.DryRun;
                var diffTable = new Table().Expand();
                diffTable.Title = new TableTitle($"[blue]DIFF: {Markup.Escape(track.Path ?? "")}[/]");
                diffTable.AddColumn("property")
                    .AddColumn("current")
                    .AddColumn("new");
                foreach (var (property, currentValue, newValue) in diffListing)
                {
                    diffTable.AddRow(
                        property.ToString(),
                        Markup.Escape(newValue?.ToString() ?? "<null>"),
                        Markup.Escape(currentValue?.ToString() ?? "<null>")
                    );
                }

                if (settings.DryRun)
                {
                    _console.Write(diffTable);
                    continue;
                }

                var path = Markup.Escape(track.Path ?? "");
                var message = !track.Save()
                    ? $"[red]Update failed: {path}[/]"
                    : $"[green]Updated: {path}[/]";
                diffTable.Caption = new TableTitle(message);
                _console.Write(diffTable);
            }
        }

        return showDryRunMessage;
    }


oder sehe ich das falsch?

16.806 Beiträge seit 2008
vor einem Jahr

hab ich noch nicht richtig verstanden...

Du hast dann nur noch einen Task, brauchst damit auch keinerlei Task.WaitAll etc...

Es gibt für das ganze auch Recommended Patterns, die so quasi heute auch noch ihre Gültigkeit haben.
Recommended patterns for CancellationToken

Idee ist immer:

  • so früh wie möglich das IsCancelled abfragen bzw ThrowIfCancellationRequested je nach Kontext zu verwenden
  • Kontinuierlich abfragen, ob mittlerweile IsCancelled eingetroffen ist (zB in einer Schleife, in Untermethoden, Operationen etc etc..)
  • Token immer weiter geben, so tief wie möglich

IsCancelled verwendet man i.d.R. wenn man aktiv auf Cancel reagieren muss (zB etwas aufräumen muss), ansonsten ThrowIfCancellationRequested, wobei man je nachdem auf für ersteres die Exception fängt (und ggfls weiter reicht).

Die anderen Themen, die Du hast, sind Task Best Practises (zB keine Kaskadierung von Parallelität etc..).

PS: bitte keine Full Quotes.

S
sandreas Themenstarter:in
29 Beiträge seit 2022
vor einem Jahr

So, erstmal vielen Dank für die Antwort, die hat sehr geholfen.

Die anderen Themen, die Du hast, sind Task Best Practises (zB keine Kaskadierung von Parallelität etc..).

Hast du da eine gute Kurz-Referenz oder anderen Lesestoff für mich (also für "Task Best Practises")?

PS: bitte keine Full Quotes.

Geht klar... das Forum macht es einem nicht unbedingt leicht damit, aber ich achte in Zukunft drauf.

16.806 Beiträge seit 2008
vor einem Jahr

Google Suche nach "c# task best practices" 🙂Async/Await - Best Practices in Asynchronous Programming

Geht klar... das Forum macht es einem nicht unbedingt leicht damit, aber ich achte in Zukunft drauf.

99,99% bekommens hin, dass sie einfach "Antworten" drücken - oder bei "Zitieren" einfach den unrelevanten Anteil entfernen.
Besser geht immer, aber denke das ist okay. 🙂

S
sandreas Themenstarter:in
29 Beiträge seit 2022
vor einem Jahr

Firma dankt. Thema ist beantwortet.