Skip to content

Добавлен параметр завершения дочерних процессов в метод Завершить() #1660

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.

Sign up for GitHub

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

Open
yukon39 wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Добавлен параметр завершения дочерних процессов в метод Завершить() #1660

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/OneScript.StandardLibrary/Processes/ProcessContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,15 @@ public int ProcessId

[ContextProperty("Имя", "Name")] public string Name => _p.ProcessName;

[ContextMethod("Завершить","Stop")]
public void Stop()
/// <summary>
/// Завершить процесс и, опционально, все его дочерние процессы.
/// </summary>
/// <param name="entireProcessTree">Булево. Завершить все дочерние процессы.</param>
/// <exception cref="AggregateException">Не все дочерние процессы удалось завершить (только при entireProcessTree = true).</exception>
[ContextMethod("Завершить", "Stop")]
public void Stop(bool entireProcessTree = false)
{
_p.Kill();
_p.Kill(entireProcessTree);
}
Comment on lines 199 to 208
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

WaitForExit/HasExited do not track descendant termination after Kill(true)

The WaitForExit method and the HasExited property do not reflect the status of descendant processes; when Kill(entireProcessTree: true) is used, they will indicate completion after the root process exits, even if all descendants have not yet exited.

This is a subtle behavioral difference worth noting in the method's XML documentation so that callers who call ОжидатьЗавершения() / WaitForExit after Завершить(Истина) are not surprised.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/OneScript.StandardLibrary/Processes/ProcessContext.cs` around lines 199 -
207, Update the XML documentation for ProcessContext.Stop (the method wrapping
_p.Kill) to explicitly state that calling Stop(true) / Завершить(Истина) kills
the root process but WaitForExit and the HasExited property do not wait for or
reflect termination of descendant processes, so callers using
ОжидатьЗавершения() / WaitForExit after Stop(true) may still observe running
child processes and should handle descendant termination separately; include
this note in the <summary> or as an additional <remarks> sentence so it's
clearly visible in generated docs.

⚠️ Potential issue | 🟡 Minor

AggregateException from Kill(true) is unhandled and undocumented

Kill(bool) can throw an AggregateException when not all processes in the associated process's descendant tree could be terminated. This occurs when Kill(true) tries to terminate child processes but lacks permissions for some of them — for example, processes owned by different users.

The XML <summary> and <param> doc do not mention this, so script callers have no warning. Consider either wrapping it or at minimum adding a <exception> doc tag.

📝 Suggested doc addition 
 /// <summary>
 /// Завершить процесс и, опционально, все его дочерние процессы.
 /// </summary>
 /// <param name="entireProcessTree">Булево. Завершить все дочерние процессы.</param>
+/// <exception cref="AggregateException">Не все дочерние процессы удалось завершить (только при entireProcessTree = true).</exception>
 [ContextMethod("Завершить", "Stop")]
 public void Stop(bool entireProcessTree = false)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/// <summary>
/// Завершить процесс и, опционально, все его дочерние процессы.
/// </summary>
/// <param name="entireProcessTree">Булево. Завершить все дочерние процессы.</param>
[ContextMethod("Завершить", "Stop")]
public void Stop(bool entireProcessTree = false)
{
_p.Kill();
_p.Kill(entireProcessTree);
}
/// <summary>
/// Завершить процесс и, опционально, все его дочерние процессы.
/// </summary>
/// <param name="entireProcessTree">Булево. Завершить все дочерние процессы.</param>
/// <exception cref="AggregateException">Не все дочерние процессы удалось завершить (только при entireProcessTree = true).</exception>
[ContextMethod("Завершить", "Stop")]
public void Stop(bool entireProcessTree = false)
{
_p.Kill(entireProcessTree);
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/OneScript.StandardLibrary/Processes/ProcessContext.cs` around lines 199 -
207, The Stop method calls _p.Kill(entireProcessTree) which can throw
AggregateException when Kill(true) cannot terminate some descendant processes
(e.g., due to permissions); update Stop to catch AggregateException from the
call to Kill and rethrow a clearer exception (e.g., throw new
InvalidOperationException("Failed to terminate one or more child processes.",
ex)) preserving the original AggregateException as InnerException, and also add
an XML <exception> entry to Stop's doc comment that documents this
AggregateException/InvalidOperationException scenario when entireProcessTree is
true; reference the Stop method and the call to Kill(bool) when making the
changes.


public void Dispose()
Expand Down