Willkommen auf myCSharp.de! Anmelden | kostenlos registrieren
 | Suche | FAQ

Hauptmenü
myCSharp.de
» Startseite
» Forum
» Suche
» Regeln
» Wie poste ich richtig?

Mitglieder
» Liste / Suche
» Wer ist online?

Ressourcen
» FAQ
» Artikel
» C#-Snippets
» Jobbörse
» Microsoft Docs

Team
» Kontakt
» Cookies
» Spenden
» Datenschutz
» Impressum

  • »
  • Community
  • |
  • Diskussionsforum
Task basierte Kommandozeilen-Applikaton "graceful" beenden
sandreas
myCSharp.de - Member



Dabei seit:
Beiträge: 29

Themenstarter:

Task basierte Kommandozeilen-Applikaton "graceful" beenden

beantworten | zitieren | melden

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?
Dieser Beitrag wurde 2 mal editiert, zum letzten Mal von sandreas am .
private Nachricht | Beiträge des Benutzers
Abt
myCSharp.de - Team

Avatar #avatar-4119.png


Dabei seit:
Beiträge: 16.209

beantworten | zitieren | melden


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.
private Nachricht | Beiträge des Benutzers
sandreas
myCSharp.de - Member



Dabei seit:
Beiträge: 29

Themenstarter:

beantworten | zitieren | melden

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

Zitat
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?
private Nachricht | Beiträge des Benutzers
Abt
myCSharp.de - Team

Avatar #avatar-4119.png


Dabei seit:
Beiträge: 16.209

beantworten | zitieren | melden

Zitat von sandreas
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.
private Nachricht | Beiträge des Benutzers
sandreas
myCSharp.de - Member



Dabei seit:
Beiträge: 29

Themenstarter:

beantworten | zitieren | melden

So, erstmal vielen Dank für die Antwort, die hat sehr geholfen.
Zitat von Abt
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")?
Zitat von Abt
PS: bitte keine Full Quotes.
Geht klar... das Forum macht es einem nicht unbedingt leicht damit, aber ich achte in Zukunft drauf.
private Nachricht | Beiträge des Benutzers
Abt
myCSharp.de - Team

Avatar #avatar-4119.png


Dabei seit:
Beiträge: 16.209

beantworten | zitieren | melden

Google Suche nach "c# task best practices" :-)
Async/Await - Best Practices in Asynchronous Programming
Zitat
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. :-)
private Nachricht | Beiträge des Benutzers
sandreas
myCSharp.de - Member



Dabei seit:
Beiträge: 29

Themenstarter:

beantworten | zitieren | melden

Firma dankt. Thema ist beantwortet.
private Nachricht | Beiträge des Benutzers