Haiku: Process/thread management functions - #121883

Open
trungnt2910 wants to merge 4 commits into
dotnet:mainfrom
trungnt2910:dev/trungnt2910/haiku-lib-diagnostics
Open

Haiku: Process/thread management functions#121883
trungnt2910 wants to merge 4 commits into
dotnet:mainfrom
trungnt2910:dev/trungnt2910/haiku-lib-diagnostics

Conversation

@trungnt2910

Copy link
Copy Markdown
Contributor

Add support for process/thread management functions in System.Diagnostics.Process for Haiku.

This is required to build managed runtime libraries for Haiku as well as running a simple "Hello, World!" application.

Part of #55803.

CopilotAI review requested due to automatic review settings November 21, 2025 15:01
@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Nov 21, 2025
@trungnt2910

Copy link
Copy Markdown
ContributorAuthor

C/c @am11

I just added the relevant parts of the common files to both, then we can rebase stuff later.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds Haiku operating system support to the System.Diagnostics.Process library, enabling process and thread management functionality for Haiku. This is part of the broader effort to support Haiku as a platform (#55803).

  • Implements Haiku-specific process/thread management APIs using native Haiku system calls
  • Adds proper platform attribute annotations for API surface compatibility
  • Integrates Haiku interop layer for process, thread, and image information

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
ProcessThread.csAdds [SupportedOSPlatform("haiku")] attributes to PriorityLevel property getter and setter
ProcessThread.Haiku.csImplements Haiku-specific thread priority management and processor time tracking using native BeOS-style priority values
ProcessManager.Haiku.csImplements process enumeration, module collection, and process information retrieval for Haiku using team and thread APIs
Process.csAdds [UnsupportedOSPlatform("haiku")] attributes to working set limit properties
Process.Haiku.csImplements Haiku-specific process properties including boot time caching, start time, processor time tracking, and platform-specific helper methods
System.Diagnostics.Process.csprojAdds Haiku target framework and includes Haiku-specific source files and interop definitions
Interop.OS.csDefines Haiku native interop for OS-level APIs including team/thread/area/system information structures and P/Invoke declarations
Interop.Libraries.csDefines the libroot library constant for Haiku native library imports
Interop.Image.csDefines Haiku native interop for image (module) information APIs

@@ -100,6 +101,7 @@ public ThreadPriorityLevel PriorityLevel
return _priorityLevel.Value;
}
[SupportedOSPlatform("windows")]

CopilotAINov 21, 2025

Copy link

Choose a reason for hiding this comment

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

The setter is missing [SupportedOSPlatform("linux")] and [SupportedOSPlatform("freebsd")] attributes. While these platforms throw PlatformNotSupportedException in their implementations, the attributes on the public API should match the getter's attributes to maintain consistency and accuracy in the API surface declarations. The presence of the getter attribute for these platforms indicates the API should be callable, even if it throws at runtime.

Suggested change
[SupportedOSPlatform("windows")]
[SupportedOSPlatform("windows")]
[SupportedOSPlatform("linux")]
[SupportedOSPlatform("freebsd")]

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Ignoring since it's not related to Haiku.

Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.OS.cs Outdated
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 3db3a8c to 822ecd1CompareNovember 21, 2025 15:28
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.Image.cs Outdated
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.Image.cs Outdated
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 822ecd1 to 7863babCompareNovember 21, 2025 15:51
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch 2 times, most recently from b7ee79c to f458cbdCompareNovember 22, 2025 02:53
@jeffhandleyjeffhandley added the needs-author-action An issue or pull request that requires more info or actions from the author. label Feb 1, 2026
@trungnt2910

Copy link
Copy Markdown
ContributorAuthor

Still tracking, will take action soon.

@dotnet-policy-servicedotnet-policy-serviceBot removed needs-author-action An issue or pull request that requires more info or actions from the author. no-recent-activity labels Feb 16, 2026
@jeffhandley

Copy link
Copy Markdown
Member

@trungnt2910 I pushed 0ee9ed3 to my fork; it merges main into your branch and resolves the conflict. Feel free to cherry-pick that commit if it's helpful.

@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from f458cbd to 5ed9cf0CompareApril 11, 2026 10:08
CopilotAI review requested due to automatic review settings April 11, 2026 10:08

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

Comment threadsrc/native/libs/configure.cmake
Comment threadsrc/native/libs/System.Native/pal_getosinfo.c
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.OS.cs
CopilotAI review requested due to automatic review settings April 11, 2026 11:23

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

Comment threadsrc/native/libs/System.Native/pal_getosinfo.c
Comment threadsrc/native/libs/configure.cmake
Comment on lines +14 to +21
public static void GetProcessInfos(ref ArrayBuilder<ProcessInfo> builder, string? processNameFilter)
{
int cookie = 0;
Interop.OS.TeamInfo info;

while ((Interop.OS.GetNextTeamInfo(ref cookie, out info)) == 0)
{
ProcessInfo? pi = GetProcessInfoFromTeamInfo(ref info, processNameFilter);

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

Interop.OS.TeamInfo is declared as an unsafe struct (due to the fixed buffer). Declaring/using it here (and calling GetNextTeamInfo) requires an unsafe context; as written this method will fail to compile for the Haiku TFM. Mark the method as unsafe (or refactor the interop type to avoid requiring unsafe at call sites).

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

Comment on lines +73 to +81
internal static ProcessInfo? CreateProcessInfo(int pid, string? processNameFilter = null)
{
// Negative PIDs aren't valid
ArgumentOutOfRangeException.ThrowIfNegative(pid);

Interop.OS.TeamInfo info;
int status = Interop.OS.GetTeamInfo(pid, out info);

if (status != 0)

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

CreateProcessInfo uses Interop.OS.TeamInfo (an unsafe struct because of the fixed buffer). This method needs to be in an unsafe context (e.g., mark it unsafe) or the interop needs to be reshaped so callers don't need unsafe; otherwise the Haiku build will fail to compile.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

Comment on lines +45 to +53
internal DateTime StartTimeCore
{
get
{
EnsureState(State.HaveNonExitedId);

Interop.OS.TeamInfo info;
int status = Interop.OS.GetTeamInfo(_processId, out info);

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

StartTimeCore uses Interop.OS.TeamInfo (an unsafe struct because of the fixed buffer). This getter must be in an unsafe context (e.g., mark the property/getter as unsafe) or the interop needs to be reshaped; otherwise the Haiku TFM will not compile.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

CopilotAI review requested due to automatic review settings April 11, 2026 11:53
@trungnt2910

trungnt2910 commented Apr 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Local builds are succeeding in both Release and Debug mode (with all my other open PRs on native components applied).

@jkotas
jkotas requested a review from adamsitnikApril 11, 2026 14:29
CopilotAI review requested due to automatic review settings April 11, 2026 14:43

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

CopilotAI review requested due to automatic review settings April 26, 2026 03:10

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

@am11

am11 commented Apr 26, 2026

Copy link
Copy Markdown
Member

LGTM!
cc @adamsitnik, PTAL.

trungnt2910and others added 3 commits April 26, 2026 22:46
Add support for process/thread management functions in
`System.Diagnostics.Process` for Haiku.
This is required to build managed runtime libraries for Haiku as well as
running a simple "Hello, World!" application.
Co-authored-by: Jessica Hamilton <jessica.l.hamilton@gmail.com>
Declare Haiku as a supported platform to MSBuild for all
`System.Diagnostics.Process` builds.
This prevents `CA1418` when using the `SupportedOSPlatform` attribute
with `haiku`.
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 50691e9 to d460910CompareApril 26, 2026 12:47
Otherwise `clang`-based builds will complain.
`clang` treats the `enum`s as unsigned `int`, triggering a sign conversion warning.
CopilotAI review requested due to automatic review settings April 26, 2026 13:18

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

@github-actions

github-actionsBot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
"version": 5,
"last_dispatched_commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "4e26169766106d54cf582a095878d6d439763335",
"last_reviewed_commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "4e26169766106d54cf582a095878d6d439763335",
"last_recorded_worker_run_id": "29674636225",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"review_id": 4730182665
}
]
}

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Holistic Review

Motivation: Justified. This adds System.Diagnostics.Process support for Haiku (part of #55803), which is a real prerequisite for building and running managed libraries on that platform. The approach follows the established per-platform partial-class pattern already used for iOS, SunOS, etc.

Approach: Sound and consistent with the codebase. New PInvokes are added as SystemNative_* entrypoints wrapped in #if HAVE_OS_H/HAVE_IMAGE_H with ENOTSUP fallbacks, structs are marshalled through PAL shims, and the managed side reuses ArrayBuilder/ProcessModuleCollection. Public API surface changes are additive [SupportedOSPlatform("haiku")] annotations on existing members, so no new API approval is required.

Summary: ⚠️ Needs Human Review. The change is well-structured and platform-isolated, so it cannot regress existing platforms. My main concern is the UTF-8 vs. ANSI string decoding of process/module names (flagged inline). There is also a dead GetNextTeamId PInvoke. Neither blocks the platform bring-up goal, but a maintainer familiar with the Haiku work should confirm the name-decoding intent.


Detailed Findings

⚠️ Correctness — Process/module name decoding uses the default code page, not UTF-8

See the inline comment on ProcessManager.Haiku.cs. new string((sbyte*)ptr, 0, length) interprets the bytes using the current ANSI code page, whereas Haiku names are UTF-8. Non-ASCII process/module names will be mangled. Decoding via Encoding.UTF8 (over the computed length) would match both the platform encoding and the convention used elsewhere for native UTF-8 strings.

💡 Dead code — SystemNative_GetNextTeamId appears unused

GetNextTeamId is declared in Interop.OS.cs, exported in entrypoints.c, and implemented in pal_getosinfo.c, but I found no managed caller (GetProcessInfos iterates via GetNextTeamInfo, and there is no EnumerateProcessIds/GetProcessIds for Haiku). If it is not needed for the current bring-up, consider removing it to avoid unused surface; if a follow-up will consume it, a brief note would help.

✅ Native PAL layer — guards and fallbacks look correct

Each entrypoint null-checks its out-params, memsets native structs before use, returns status_t codes directly, and provides (void)-cast ENOTSUP stubs when HAVE_OS_H/HAVE_IMAGE_H are absent. c_static_assert(SYSTEMNATIVE_MAX_PATH >= MAXPATHLEN) guards the image-name buffer. The struct field layouts match the managed [StructLayout(LayoutKind.Sequential)] definitions.

✅ Boot-time caching — race-safe

Process.Haiku.cs caches s_bootTimeTicks with Interlocked.Read/CompareExchange and reconciles with the winning writer's value, so concurrent initialization is handled correctly.

✅ Test coverage — no new tests, but consistent with platform bring-up

No unit tests are added, which is consistent with other platform-enablement PRs in this project where cross-platform System.Diagnostics.Process tests provide coverage once the platform is buildable. A human reviewer should confirm this matches the project's expectations for Haiku enablement.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 181.6 AIC · ⌖ 11 AIC · ⊞ 10K

private static unsafe string GetString(byte* ptr, int maxLength)
{
int length = new ReadOnlySpan<byte>(ptr, maxLength).IndexOf((byte)0);
return new string((sbyte*)ptr, 0, (length < 0) ? maxLength : length);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️new string((sbyte*)ptr, 0, length) decodes the bytes using the system default ANSI code page rather than UTF-8. Haiku uses UTF-8 for team and image names, so any process or module name containing non-ASCII characters will be misdecoded here. Consider Encoding.UTF8.GetString(new ReadOnlySpan<byte>(ptr, (length < 0) ? maxLength : length)) (you already compute the NUL-terminated length just above), which is the pattern used elsewhere in the codebase for UTF-8 native strings.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Diagnostics.Processcommunity-contributionIndicates that the PR has been added by a community memberos-haiku

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@trungnt2910@jeffhandley@am11@huoyaoyuan@jkotas
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Haiku: Process/thread management functions - #121883

Open
trungnt2910 wants to merge 4 commits into
dotnet:mainfrom
trungnt2910:dev/trungnt2910/haiku-lib-diagnostics
Open

Haiku: Process/thread management functions#121883
trungnt2910 wants to merge 4 commits into
dotnet:mainfrom
trungnt2910:dev/trungnt2910/haiku-lib-diagnostics

Conversation

@trungnt2910

Copy link
Copy Markdown
Contributor

Add support for process/thread management functions in System.Diagnostics.Process for Haiku.

This is required to build managed runtime libraries for Haiku as well as running a simple "Hello, World!" application.

Part of #55803.

CopilotAI review requested due to automatic review settings November 21, 2025 15:01
@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Nov 21, 2025
@trungnt2910

Copy link
Copy Markdown
ContributorAuthor

C/c @am11

I just added the relevant parts of the common files to both, then we can rebase stuff later.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds Haiku operating system support to the System.Diagnostics.Process library, enabling process and thread management functionality for Haiku. This is part of the broader effort to support Haiku as a platform (#55803).

  • Implements Haiku-specific process/thread management APIs using native Haiku system calls
  • Adds proper platform attribute annotations for API surface compatibility
  • Integrates Haiku interop layer for process, thread, and image information

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
ProcessThread.csAdds [SupportedOSPlatform("haiku")] attributes to PriorityLevel property getter and setter
ProcessThread.Haiku.csImplements Haiku-specific thread priority management and processor time tracking using native BeOS-style priority values
ProcessManager.Haiku.csImplements process enumeration, module collection, and process information retrieval for Haiku using team and thread APIs
Process.csAdds [UnsupportedOSPlatform("haiku")] attributes to working set limit properties
Process.Haiku.csImplements Haiku-specific process properties including boot time caching, start time, processor time tracking, and platform-specific helper methods
System.Diagnostics.Process.csprojAdds Haiku target framework and includes Haiku-specific source files and interop definitions
Interop.OS.csDefines Haiku native interop for OS-level APIs including team/thread/area/system information structures and P/Invoke declarations
Interop.Libraries.csDefines the libroot library constant for Haiku native library imports
Interop.Image.csDefines Haiku native interop for image (module) information APIs

@@ -100,6 +101,7 @@ public ThreadPriorityLevel PriorityLevel
return _priorityLevel.Value;
}
[SupportedOSPlatform("windows")]

CopilotAINov 21, 2025

Copy link

Choose a reason for hiding this comment

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

The setter is missing [SupportedOSPlatform("linux")] and [SupportedOSPlatform("freebsd")] attributes. While these platforms throw PlatformNotSupportedException in their implementations, the attributes on the public API should match the getter's attributes to maintain consistency and accuracy in the API surface declarations. The presence of the getter attribute for these platforms indicates the API should be callable, even if it throws at runtime.

Suggested change
[SupportedOSPlatform("windows")]
[SupportedOSPlatform("windows")]
[SupportedOSPlatform("linux")]
[SupportedOSPlatform("freebsd")]

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Ignoring since it's not related to Haiku.

Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.OS.cs Outdated
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 3db3a8c to 822ecd1CompareNovember 21, 2025 15:28
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.Image.cs Outdated
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.Image.cs Outdated
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 822ecd1 to 7863babCompareNovember 21, 2025 15:51
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch 2 times, most recently from b7ee79c to f458cbdCompareNovember 22, 2025 02:53
@jeffhandleyjeffhandley added the needs-author-action An issue or pull request that requires more info or actions from the author. label Feb 1, 2026
@trungnt2910

Copy link
Copy Markdown
ContributorAuthor

Still tracking, will take action soon.

@dotnet-policy-servicedotnet-policy-serviceBot removed needs-author-action An issue or pull request that requires more info or actions from the author. no-recent-activity labels Feb 16, 2026
@jeffhandley

Copy link
Copy Markdown
Member

@trungnt2910 I pushed 0ee9ed3 to my fork; it merges main into your branch and resolves the conflict. Feel free to cherry-pick that commit if it's helpful.

@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from f458cbd to 5ed9cf0CompareApril 11, 2026 10:08
CopilotAI review requested due to automatic review settings April 11, 2026 10:08

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

Comment threadsrc/native/libs/configure.cmake
Comment threadsrc/native/libs/System.Native/pal_getosinfo.c
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.OS.cs
CopilotAI review requested due to automatic review settings April 11, 2026 11:23

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

Comment threadsrc/native/libs/System.Native/pal_getosinfo.c
Comment threadsrc/native/libs/configure.cmake
Comment on lines +14 to +21
public static void GetProcessInfos(ref ArrayBuilder<ProcessInfo> builder, string? processNameFilter)
{
int cookie = 0;
Interop.OS.TeamInfo info;

while ((Interop.OS.GetNextTeamInfo(ref cookie, out info)) == 0)
{
ProcessInfo? pi = GetProcessInfoFromTeamInfo(ref info, processNameFilter);

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

Interop.OS.TeamInfo is declared as an unsafe struct (due to the fixed buffer). Declaring/using it here (and calling GetNextTeamInfo) requires an unsafe context; as written this method will fail to compile for the Haiku TFM. Mark the method as unsafe (or refactor the interop type to avoid requiring unsafe at call sites).

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

Comment on lines +73 to +81
internal static ProcessInfo? CreateProcessInfo(int pid, string? processNameFilter = null)
{
// Negative PIDs aren't valid
ArgumentOutOfRangeException.ThrowIfNegative(pid);

Interop.OS.TeamInfo info;
int status = Interop.OS.GetTeamInfo(pid, out info);

if (status != 0)

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

CreateProcessInfo uses Interop.OS.TeamInfo (an unsafe struct because of the fixed buffer). This method needs to be in an unsafe context (e.g., mark it unsafe) or the interop needs to be reshaped so callers don't need unsafe; otherwise the Haiku build will fail to compile.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

Comment on lines +45 to +53
internal DateTime StartTimeCore
{
get
{
EnsureState(State.HaveNonExitedId);

Interop.OS.TeamInfo info;
int status = Interop.OS.GetTeamInfo(_processId, out info);

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

StartTimeCore uses Interop.OS.TeamInfo (an unsafe struct because of the fixed buffer). This getter must be in an unsafe context (e.g., mark the property/getter as unsafe) or the interop needs to be reshaped; otherwise the Haiku TFM will not compile.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

CopilotAI review requested due to automatic review settings April 11, 2026 11:53
@trungnt2910

trungnt2910 commented Apr 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Local builds are succeeding in both Release and Debug mode (with all my other open PRs on native components applied).

@jkotas
jkotas requested a review from adamsitnikApril 11, 2026 14:29
CopilotAI review requested due to automatic review settings April 11, 2026 14:43

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

CopilotAI review requested due to automatic review settings April 26, 2026 03:10

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

@am11

am11 commented Apr 26, 2026

Copy link
Copy Markdown
Member

LGTM!
cc @adamsitnik, PTAL.

trungnt2910and others added 3 commits April 26, 2026 22:46
Add support for process/thread management functions in
`System.Diagnostics.Process` for Haiku.
This is required to build managed runtime libraries for Haiku as well as
running a simple "Hello, World!" application.
Co-authored-by: Jessica Hamilton <jessica.l.hamilton@gmail.com>
Declare Haiku as a supported platform to MSBuild for all
`System.Diagnostics.Process` builds.
This prevents `CA1418` when using the `SupportedOSPlatform` attribute
with `haiku`.
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 50691e9 to d460910CompareApril 26, 2026 12:47
Otherwise `clang`-based builds will complain.
`clang` treats the `enum`s as unsigned `int`, triggering a sign conversion warning.
CopilotAI review requested due to automatic review settings April 26, 2026 13:18

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

@github-actions

github-actionsBot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
"version": 5,
"last_dispatched_commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "4e26169766106d54cf582a095878d6d439763335",
"last_reviewed_commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "4e26169766106d54cf582a095878d6d439763335",
"last_recorded_worker_run_id": "29674636225",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"review_id": 4730182665
}
]
}

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Holistic Review

Motivation: Justified. This adds System.Diagnostics.Process support for Haiku (part of #55803), which is a real prerequisite for building and running managed libraries on that platform. The approach follows the established per-platform partial-class pattern already used for iOS, SunOS, etc.

Approach: Sound and consistent with the codebase. New PInvokes are added as SystemNative_* entrypoints wrapped in #if HAVE_OS_H/HAVE_IMAGE_H with ENOTSUP fallbacks, structs are marshalled through PAL shims, and the managed side reuses ArrayBuilder/ProcessModuleCollection. Public API surface changes are additive [SupportedOSPlatform("haiku")] annotations on existing members, so no new API approval is required.

Summary: ⚠️ Needs Human Review. The change is well-structured and platform-isolated, so it cannot regress existing platforms. My main concern is the UTF-8 vs. ANSI string decoding of process/module names (flagged inline). There is also a dead GetNextTeamId PInvoke. Neither blocks the platform bring-up goal, but a maintainer familiar with the Haiku work should confirm the name-decoding intent.


Detailed Findings

⚠️ Correctness — Process/module name decoding uses the default code page, not UTF-8

See the inline comment on ProcessManager.Haiku.cs. new string((sbyte*)ptr, 0, length) interprets the bytes using the current ANSI code page, whereas Haiku names are UTF-8. Non-ASCII process/module names will be mangled. Decoding via Encoding.UTF8 (over the computed length) would match both the platform encoding and the convention used elsewhere for native UTF-8 strings.

💡 Dead code — SystemNative_GetNextTeamId appears unused

GetNextTeamId is declared in Interop.OS.cs, exported in entrypoints.c, and implemented in pal_getosinfo.c, but I found no managed caller (GetProcessInfos iterates via GetNextTeamInfo, and there is no EnumerateProcessIds/GetProcessIds for Haiku). If it is not needed for the current bring-up, consider removing it to avoid unused surface; if a follow-up will consume it, a brief note would help.

✅ Native PAL layer — guards and fallbacks look correct

Each entrypoint null-checks its out-params, memsets native structs before use, returns status_t codes directly, and provides (void)-cast ENOTSUP stubs when HAVE_OS_H/HAVE_IMAGE_H are absent. c_static_assert(SYSTEMNATIVE_MAX_PATH >= MAXPATHLEN) guards the image-name buffer. The struct field layouts match the managed [StructLayout(LayoutKind.Sequential)] definitions.

✅ Boot-time caching — race-safe

Process.Haiku.cs caches s_bootTimeTicks with Interlocked.Read/CompareExchange and reconciles with the winning writer's value, so concurrent initialization is handled correctly.

✅ Test coverage — no new tests, but consistent with platform bring-up

No unit tests are added, which is consistent with other platform-enablement PRs in this project where cross-platform System.Diagnostics.Process tests provide coverage once the platform is buildable. A human reviewer should confirm this matches the project's expectations for Haiku enablement.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 181.6 AIC · ⌖ 11 AIC · ⊞ 10K

private static unsafe string GetString(byte* ptr, int maxLength)
{
int length = new ReadOnlySpan<byte>(ptr, maxLength).IndexOf((byte)0);
return new string((sbyte*)ptr, 0, (length < 0) ? maxLength : length);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️new string((sbyte*)ptr, 0, length) decodes the bytes using the system default ANSI code page rather than UTF-8. Haiku uses UTF-8 for team and image names, so any process or module name containing non-ASCII characters will be misdecoded here. Consider Encoding.UTF8.GetString(new ReadOnlySpan<byte>(ptr, (length < 0) ? maxLength : length)) (you already compute the NUL-terminated length just above), which is the pattern used elsewhere in the codebase for UTF-8 native strings.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Diagnostics.Processcommunity-contributionIndicates that the PR has been added by a community memberos-haiku

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@trungnt2910@jeffhandley@am11@huoyaoyuan@jkotas
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Haiku: Process/thread management functions - #121883

Open
trungnt2910 wants to merge 4 commits into
dotnet:mainfrom
trungnt2910:dev/trungnt2910/haiku-lib-diagnostics
Open

Haiku: Process/thread management functions#121883
trungnt2910 wants to merge 4 commits into
dotnet:mainfrom
trungnt2910:dev/trungnt2910/haiku-lib-diagnostics

Conversation

@trungnt2910

Copy link
Copy Markdown
Contributor

Add support for process/thread management functions in System.Diagnostics.Process for Haiku.

This is required to build managed runtime libraries for Haiku as well as running a simple "Hello, World!" application.

Part of #55803.

CopilotAI review requested due to automatic review settings November 21, 2025 15:01
@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Nov 21, 2025
@trungnt2910

Copy link
Copy Markdown
ContributorAuthor

C/c @am11

I just added the relevant parts of the common files to both, then we can rebase stuff later.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds Haiku operating system support to the System.Diagnostics.Process library, enabling process and thread management functionality for Haiku. This is part of the broader effort to support Haiku as a platform (#55803).

  • Implements Haiku-specific process/thread management APIs using native Haiku system calls
  • Adds proper platform attribute annotations for API surface compatibility
  • Integrates Haiku interop layer for process, thread, and image information

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
ProcessThread.csAdds [SupportedOSPlatform("haiku")] attributes to PriorityLevel property getter and setter
ProcessThread.Haiku.csImplements Haiku-specific thread priority management and processor time tracking using native BeOS-style priority values
ProcessManager.Haiku.csImplements process enumeration, module collection, and process information retrieval for Haiku using team and thread APIs
Process.csAdds [UnsupportedOSPlatform("haiku")] attributes to working set limit properties
Process.Haiku.csImplements Haiku-specific process properties including boot time caching, start time, processor time tracking, and platform-specific helper methods
System.Diagnostics.Process.csprojAdds Haiku target framework and includes Haiku-specific source files and interop definitions
Interop.OS.csDefines Haiku native interop for OS-level APIs including team/thread/area/system information structures and P/Invoke declarations
Interop.Libraries.csDefines the libroot library constant for Haiku native library imports
Interop.Image.csDefines Haiku native interop for image (module) information APIs

@@ -100,6 +101,7 @@ public ThreadPriorityLevel PriorityLevel
return _priorityLevel.Value;
}
[SupportedOSPlatform("windows")]

CopilotAINov 21, 2025

Copy link

Choose a reason for hiding this comment

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

The setter is missing [SupportedOSPlatform("linux")] and [SupportedOSPlatform("freebsd")] attributes. While these platforms throw PlatformNotSupportedException in their implementations, the attributes on the public API should match the getter's attributes to maintain consistency and accuracy in the API surface declarations. The presence of the getter attribute for these platforms indicates the API should be callable, even if it throws at runtime.

Suggested change
[SupportedOSPlatform("windows")]
[SupportedOSPlatform("windows")]
[SupportedOSPlatform("linux")]
[SupportedOSPlatform("freebsd")]

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Ignoring since it's not related to Haiku.

Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.OS.cs Outdated
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 3db3a8c to 822ecd1CompareNovember 21, 2025 15:28
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.Image.cs Outdated
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.Image.cs Outdated
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 822ecd1 to 7863babCompareNovember 21, 2025 15:51
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch 2 times, most recently from b7ee79c to f458cbdCompareNovember 22, 2025 02:53
@jeffhandleyjeffhandley added the needs-author-action An issue or pull request that requires more info or actions from the author. label Feb 1, 2026
@trungnt2910

Copy link
Copy Markdown
ContributorAuthor

Still tracking, will take action soon.

@dotnet-policy-servicedotnet-policy-serviceBot removed needs-author-action An issue or pull request that requires more info or actions from the author. no-recent-activity labels Feb 16, 2026
@jeffhandley

Copy link
Copy Markdown
Member

@trungnt2910 I pushed 0ee9ed3 to my fork; it merges main into your branch and resolves the conflict. Feel free to cherry-pick that commit if it's helpful.

@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from f458cbd to 5ed9cf0CompareApril 11, 2026 10:08
CopilotAI review requested due to automatic review settings April 11, 2026 10:08

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

Comment threadsrc/native/libs/configure.cmake
Comment threadsrc/native/libs/System.Native/pal_getosinfo.c
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.OS.cs
CopilotAI review requested due to automatic review settings April 11, 2026 11:23

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

Comment threadsrc/native/libs/System.Native/pal_getosinfo.c
Comment threadsrc/native/libs/configure.cmake
Comment on lines +14 to +21
public static void GetProcessInfos(ref ArrayBuilder<ProcessInfo> builder, string? processNameFilter)
{
int cookie = 0;
Interop.OS.TeamInfo info;

while ((Interop.OS.GetNextTeamInfo(ref cookie, out info)) == 0)
{
ProcessInfo? pi = GetProcessInfoFromTeamInfo(ref info, processNameFilter);

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

Interop.OS.TeamInfo is declared as an unsafe struct (due to the fixed buffer). Declaring/using it here (and calling GetNextTeamInfo) requires an unsafe context; as written this method will fail to compile for the Haiku TFM. Mark the method as unsafe (or refactor the interop type to avoid requiring unsafe at call sites).

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

Comment on lines +73 to +81
internal static ProcessInfo? CreateProcessInfo(int pid, string? processNameFilter = null)
{
// Negative PIDs aren't valid
ArgumentOutOfRangeException.ThrowIfNegative(pid);

Interop.OS.TeamInfo info;
int status = Interop.OS.GetTeamInfo(pid, out info);

if (status != 0)

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

CreateProcessInfo uses Interop.OS.TeamInfo (an unsafe struct because of the fixed buffer). This method needs to be in an unsafe context (e.g., mark it unsafe) or the interop needs to be reshaped so callers don't need unsafe; otherwise the Haiku build will fail to compile.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

Comment on lines +45 to +53
internal DateTime StartTimeCore
{
get
{
EnsureState(State.HaveNonExitedId);

Interop.OS.TeamInfo info;
int status = Interop.OS.GetTeamInfo(_processId, out info);

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

StartTimeCore uses Interop.OS.TeamInfo (an unsafe struct because of the fixed buffer). This getter must be in an unsafe context (e.g., mark the property/getter as unsafe) or the interop needs to be reshaped; otherwise the Haiku TFM will not compile.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

CopilotAI review requested due to automatic review settings April 11, 2026 11:53
@trungnt2910

trungnt2910 commented Apr 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Local builds are succeeding in both Release and Debug mode (with all my other open PRs on native components applied).

@jkotas
jkotas requested a review from adamsitnikApril 11, 2026 14:29
CopilotAI review requested due to automatic review settings April 11, 2026 14:43

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

CopilotAI review requested due to automatic review settings April 26, 2026 03:10

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

@am11

am11 commented Apr 26, 2026

Copy link
Copy Markdown
Member

LGTM!
cc @adamsitnik, PTAL.

trungnt2910and others added 3 commits April 26, 2026 22:46
Add support for process/thread management functions in
`System.Diagnostics.Process` for Haiku.
This is required to build managed runtime libraries for Haiku as well as
running a simple "Hello, World!" application.
Co-authored-by: Jessica Hamilton <jessica.l.hamilton@gmail.com>
Declare Haiku as a supported platform to MSBuild for all
`System.Diagnostics.Process` builds.
This prevents `CA1418` when using the `SupportedOSPlatform` attribute
with `haiku`.
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 50691e9 to d460910CompareApril 26, 2026 12:47
Otherwise `clang`-based builds will complain.
`clang` treats the `enum`s as unsigned `int`, triggering a sign conversion warning.
CopilotAI review requested due to automatic review settings April 26, 2026 13:18

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

@github-actions

github-actionsBot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
"version": 5,
"last_dispatched_commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "4e26169766106d54cf582a095878d6d439763335",
"last_reviewed_commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "4e26169766106d54cf582a095878d6d439763335",
"last_recorded_worker_run_id": "29674636225",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"review_id": 4730182665
}
]
}

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Holistic Review

Motivation: Justified. This adds System.Diagnostics.Process support for Haiku (part of #55803), which is a real prerequisite for building and running managed libraries on that platform. The approach follows the established per-platform partial-class pattern already used for iOS, SunOS, etc.

Approach: Sound and consistent with the codebase. New PInvokes are added as SystemNative_* entrypoints wrapped in #if HAVE_OS_H/HAVE_IMAGE_H with ENOTSUP fallbacks, structs are marshalled through PAL shims, and the managed side reuses ArrayBuilder/ProcessModuleCollection. Public API surface changes are additive [SupportedOSPlatform("haiku")] annotations on existing members, so no new API approval is required.

Summary: ⚠️ Needs Human Review. The change is well-structured and platform-isolated, so it cannot regress existing platforms. My main concern is the UTF-8 vs. ANSI string decoding of process/module names (flagged inline). There is also a dead GetNextTeamId PInvoke. Neither blocks the platform bring-up goal, but a maintainer familiar with the Haiku work should confirm the name-decoding intent.


Detailed Findings

⚠️ Correctness — Process/module name decoding uses the default code page, not UTF-8

See the inline comment on ProcessManager.Haiku.cs. new string((sbyte*)ptr, 0, length) interprets the bytes using the current ANSI code page, whereas Haiku names are UTF-8. Non-ASCII process/module names will be mangled. Decoding via Encoding.UTF8 (over the computed length) would match both the platform encoding and the convention used elsewhere for native UTF-8 strings.

💡 Dead code — SystemNative_GetNextTeamId appears unused

GetNextTeamId is declared in Interop.OS.cs, exported in entrypoints.c, and implemented in pal_getosinfo.c, but I found no managed caller (GetProcessInfos iterates via GetNextTeamInfo, and there is no EnumerateProcessIds/GetProcessIds for Haiku). If it is not needed for the current bring-up, consider removing it to avoid unused surface; if a follow-up will consume it, a brief note would help.

✅ Native PAL layer — guards and fallbacks look correct

Each entrypoint null-checks its out-params, memsets native structs before use, returns status_t codes directly, and provides (void)-cast ENOTSUP stubs when HAVE_OS_H/HAVE_IMAGE_H are absent. c_static_assert(SYSTEMNATIVE_MAX_PATH >= MAXPATHLEN) guards the image-name buffer. The struct field layouts match the managed [StructLayout(LayoutKind.Sequential)] definitions.

✅ Boot-time caching — race-safe

Process.Haiku.cs caches s_bootTimeTicks with Interlocked.Read/CompareExchange and reconciles with the winning writer's value, so concurrent initialization is handled correctly.

✅ Test coverage — no new tests, but consistent with platform bring-up

No unit tests are added, which is consistent with other platform-enablement PRs in this project where cross-platform System.Diagnostics.Process tests provide coverage once the platform is buildable. A human reviewer should confirm this matches the project's expectations for Haiku enablement.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 181.6 AIC · ⌖ 11 AIC · ⊞ 10K

private static unsafe string GetString(byte* ptr, int maxLength)
{
int length = new ReadOnlySpan<byte>(ptr, maxLength).IndexOf((byte)0);
return new string((sbyte*)ptr, 0, (length < 0) ? maxLength : length);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️new string((sbyte*)ptr, 0, length) decodes the bytes using the system default ANSI code page rather than UTF-8. Haiku uses UTF-8 for team and image names, so any process or module name containing non-ASCII characters will be misdecoded here. Consider Encoding.UTF8.GetString(new ReadOnlySpan<byte>(ptr, (length < 0) ? maxLength : length)) (you already compute the NUL-terminated length just above), which is the pattern used elsewhere in the codebase for UTF-8 native strings.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Diagnostics.Processcommunity-contributionIndicates that the PR has been added by a community memberos-haiku

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@trungnt2910@jeffhandley@am11@huoyaoyuan@jkotas
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Haiku: Process/thread management functions - #121883

Open
trungnt2910 wants to merge 4 commits into
dotnet:mainfrom
trungnt2910:dev/trungnt2910/haiku-lib-diagnostics
Open

Haiku: Process/thread management functions#121883
trungnt2910 wants to merge 4 commits into
dotnet:mainfrom
trungnt2910:dev/trungnt2910/haiku-lib-diagnostics

Conversation

@trungnt2910

Copy link
Copy Markdown
Contributor

Add support for process/thread management functions in System.Diagnostics.Process for Haiku.

This is required to build managed runtime libraries for Haiku as well as running a simple "Hello, World!" application.

Part of #55803.

CopilotAI review requested due to automatic review settings November 21, 2025 15:01
@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Nov 21, 2025
@trungnt2910

Copy link
Copy Markdown
ContributorAuthor

C/c @am11

I just added the relevant parts of the common files to both, then we can rebase stuff later.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds Haiku operating system support to the System.Diagnostics.Process library, enabling process and thread management functionality for Haiku. This is part of the broader effort to support Haiku as a platform (#55803).

  • Implements Haiku-specific process/thread management APIs using native Haiku system calls
  • Adds proper platform attribute annotations for API surface compatibility
  • Integrates Haiku interop layer for process, thread, and image information

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
ProcessThread.csAdds [SupportedOSPlatform("haiku")] attributes to PriorityLevel property getter and setter
ProcessThread.Haiku.csImplements Haiku-specific thread priority management and processor time tracking using native BeOS-style priority values
ProcessManager.Haiku.csImplements process enumeration, module collection, and process information retrieval for Haiku using team and thread APIs
Process.csAdds [UnsupportedOSPlatform("haiku")] attributes to working set limit properties
Process.Haiku.csImplements Haiku-specific process properties including boot time caching, start time, processor time tracking, and platform-specific helper methods
System.Diagnostics.Process.csprojAdds Haiku target framework and includes Haiku-specific source files and interop definitions
Interop.OS.csDefines Haiku native interop for OS-level APIs including team/thread/area/system information structures and P/Invoke declarations
Interop.Libraries.csDefines the libroot library constant for Haiku native library imports
Interop.Image.csDefines Haiku native interop for image (module) information APIs

@@ -100,6 +101,7 @@ public ThreadPriorityLevel PriorityLevel
return _priorityLevel.Value;
}
[SupportedOSPlatform("windows")]

CopilotAINov 21, 2025

Copy link

Choose a reason for hiding this comment

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

The setter is missing [SupportedOSPlatform("linux")] and [SupportedOSPlatform("freebsd")] attributes. While these platforms throw PlatformNotSupportedException in their implementations, the attributes on the public API should match the getter's attributes to maintain consistency and accuracy in the API surface declarations. The presence of the getter attribute for these platforms indicates the API should be callable, even if it throws at runtime.

Suggested change
[SupportedOSPlatform("windows")]
[SupportedOSPlatform("windows")]
[SupportedOSPlatform("linux")]
[SupportedOSPlatform("freebsd")]

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Ignoring since it's not related to Haiku.

Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.OS.cs Outdated
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 3db3a8c to 822ecd1CompareNovember 21, 2025 15:28
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.Image.cs Outdated
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.Image.cs Outdated
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 822ecd1 to 7863babCompareNovember 21, 2025 15:51
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch 2 times, most recently from b7ee79c to f458cbdCompareNovember 22, 2025 02:53
@jeffhandleyjeffhandley added the needs-author-action An issue or pull request that requires more info or actions from the author. label Feb 1, 2026
@trungnt2910

Copy link
Copy Markdown
ContributorAuthor

Still tracking, will take action soon.

@dotnet-policy-servicedotnet-policy-serviceBot removed needs-author-action An issue or pull request that requires more info or actions from the author. no-recent-activity labels Feb 16, 2026
@jeffhandley

Copy link
Copy Markdown
Member

@trungnt2910 I pushed 0ee9ed3 to my fork; it merges main into your branch and resolves the conflict. Feel free to cherry-pick that commit if it's helpful.

@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from f458cbd to 5ed9cf0CompareApril 11, 2026 10:08
CopilotAI review requested due to automatic review settings April 11, 2026 10:08

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

Comment threadsrc/native/libs/configure.cmake
Comment threadsrc/native/libs/System.Native/pal_getosinfo.c
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.OS.cs
CopilotAI review requested due to automatic review settings April 11, 2026 11:23

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

Comment threadsrc/native/libs/System.Native/pal_getosinfo.c
Comment threadsrc/native/libs/configure.cmake
Comment on lines +14 to +21
public static void GetProcessInfos(ref ArrayBuilder<ProcessInfo> builder, string? processNameFilter)
{
int cookie = 0;
Interop.OS.TeamInfo info;

while ((Interop.OS.GetNextTeamInfo(ref cookie, out info)) == 0)
{
ProcessInfo? pi = GetProcessInfoFromTeamInfo(ref info, processNameFilter);

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

Interop.OS.TeamInfo is declared as an unsafe struct (due to the fixed buffer). Declaring/using it here (and calling GetNextTeamInfo) requires an unsafe context; as written this method will fail to compile for the Haiku TFM. Mark the method as unsafe (or refactor the interop type to avoid requiring unsafe at call sites).

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

Comment on lines +73 to +81
internal static ProcessInfo? CreateProcessInfo(int pid, string? processNameFilter = null)
{
// Negative PIDs aren't valid
ArgumentOutOfRangeException.ThrowIfNegative(pid);

Interop.OS.TeamInfo info;
int status = Interop.OS.GetTeamInfo(pid, out info);

if (status != 0)

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

CreateProcessInfo uses Interop.OS.TeamInfo (an unsafe struct because of the fixed buffer). This method needs to be in an unsafe context (e.g., mark it unsafe) or the interop needs to be reshaped so callers don't need unsafe; otherwise the Haiku build will fail to compile.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

Comment on lines +45 to +53
internal DateTime StartTimeCore
{
get
{
EnsureState(State.HaveNonExitedId);

Interop.OS.TeamInfo info;
int status = Interop.OS.GetTeamInfo(_processId, out info);

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

StartTimeCore uses Interop.OS.TeamInfo (an unsafe struct because of the fixed buffer). This getter must be in an unsafe context (e.g., mark the property/getter as unsafe) or the interop needs to be reshaped; otherwise the Haiku TFM will not compile.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

CopilotAI review requested due to automatic review settings April 11, 2026 11:53
@trungnt2910

trungnt2910 commented Apr 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Local builds are succeeding in both Release and Debug mode (with all my other open PRs on native components applied).

@jkotas
jkotas requested a review from adamsitnikApril 11, 2026 14:29
CopilotAI review requested due to automatic review settings April 11, 2026 14:43

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

CopilotAI review requested due to automatic review settings April 26, 2026 03:10

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

@am11

am11 commented Apr 26, 2026

Copy link
Copy Markdown
Member

LGTM!
cc @adamsitnik, PTAL.

trungnt2910and others added 3 commits April 26, 2026 22:46
Add support for process/thread management functions in
`System.Diagnostics.Process` for Haiku.
This is required to build managed runtime libraries for Haiku as well as
running a simple "Hello, World!" application.
Co-authored-by: Jessica Hamilton <jessica.l.hamilton@gmail.com>
Declare Haiku as a supported platform to MSBuild for all
`System.Diagnostics.Process` builds.
This prevents `CA1418` when using the `SupportedOSPlatform` attribute
with `haiku`.
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 50691e9 to d460910CompareApril 26, 2026 12:47
Otherwise `clang`-based builds will complain.
`clang` treats the `enum`s as unsigned `int`, triggering a sign conversion warning.
CopilotAI review requested due to automatic review settings April 26, 2026 13:18

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

@github-actions

github-actionsBot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
"version": 5,
"last_dispatched_commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "4e26169766106d54cf582a095878d6d439763335",
"last_reviewed_commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "4e26169766106d54cf582a095878d6d439763335",
"last_recorded_worker_run_id": "29674636225",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"review_id": 4730182665
}
]
}

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Holistic Review

Motivation: Justified. This adds System.Diagnostics.Process support for Haiku (part of #55803), which is a real prerequisite for building and running managed libraries on that platform. The approach follows the established per-platform partial-class pattern already used for iOS, SunOS, etc.

Approach: Sound and consistent with the codebase. New PInvokes are added as SystemNative_* entrypoints wrapped in #if HAVE_OS_H/HAVE_IMAGE_H with ENOTSUP fallbacks, structs are marshalled through PAL shims, and the managed side reuses ArrayBuilder/ProcessModuleCollection. Public API surface changes are additive [SupportedOSPlatform("haiku")] annotations on existing members, so no new API approval is required.

Summary: ⚠️ Needs Human Review. The change is well-structured and platform-isolated, so it cannot regress existing platforms. My main concern is the UTF-8 vs. ANSI string decoding of process/module names (flagged inline). There is also a dead GetNextTeamId PInvoke. Neither blocks the platform bring-up goal, but a maintainer familiar with the Haiku work should confirm the name-decoding intent.


Detailed Findings

⚠️ Correctness — Process/module name decoding uses the default code page, not UTF-8

See the inline comment on ProcessManager.Haiku.cs. new string((sbyte*)ptr, 0, length) interprets the bytes using the current ANSI code page, whereas Haiku names are UTF-8. Non-ASCII process/module names will be mangled. Decoding via Encoding.UTF8 (over the computed length) would match both the platform encoding and the convention used elsewhere for native UTF-8 strings.

💡 Dead code — SystemNative_GetNextTeamId appears unused

GetNextTeamId is declared in Interop.OS.cs, exported in entrypoints.c, and implemented in pal_getosinfo.c, but I found no managed caller (GetProcessInfos iterates via GetNextTeamInfo, and there is no EnumerateProcessIds/GetProcessIds for Haiku). If it is not needed for the current bring-up, consider removing it to avoid unused surface; if a follow-up will consume it, a brief note would help.

✅ Native PAL layer — guards and fallbacks look correct

Each entrypoint null-checks its out-params, memsets native structs before use, returns status_t codes directly, and provides (void)-cast ENOTSUP stubs when HAVE_OS_H/HAVE_IMAGE_H are absent. c_static_assert(SYSTEMNATIVE_MAX_PATH >= MAXPATHLEN) guards the image-name buffer. The struct field layouts match the managed [StructLayout(LayoutKind.Sequential)] definitions.

✅ Boot-time caching — race-safe

Process.Haiku.cs caches s_bootTimeTicks with Interlocked.Read/CompareExchange and reconciles with the winning writer's value, so concurrent initialization is handled correctly.

✅ Test coverage — no new tests, but consistent with platform bring-up

No unit tests are added, which is consistent with other platform-enablement PRs in this project where cross-platform System.Diagnostics.Process tests provide coverage once the platform is buildable. A human reviewer should confirm this matches the project's expectations for Haiku enablement.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 181.6 AIC · ⌖ 11 AIC · ⊞ 10K

private static unsafe string GetString(byte* ptr, int maxLength)
{
int length = new ReadOnlySpan<byte>(ptr, maxLength).IndexOf((byte)0);
return new string((sbyte*)ptr, 0, (length < 0) ? maxLength : length);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️new string((sbyte*)ptr, 0, length) decodes the bytes using the system default ANSI code page rather than UTF-8. Haiku uses UTF-8 for team and image names, so any process or module name containing non-ASCII characters will be misdecoded here. Consider Encoding.UTF8.GetString(new ReadOnlySpan<byte>(ptr, (length < 0) ? maxLength : length)) (you already compute the NUL-terminated length just above), which is the pattern used elsewhere in the codebase for UTF-8 native strings.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Diagnostics.Processcommunity-contributionIndicates that the PR has been added by a community memberos-haiku

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@trungnt2910@jeffhandley@am11@huoyaoyuan@jkotas
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Haiku: Process/thread management functions - #121883

Open
trungnt2910 wants to merge 4 commits into
dotnet:mainfrom
trungnt2910:dev/trungnt2910/haiku-lib-diagnostics
Open

Haiku: Process/thread management functions#121883
trungnt2910 wants to merge 4 commits into
dotnet:mainfrom
trungnt2910:dev/trungnt2910/haiku-lib-diagnostics

Conversation

@trungnt2910

Copy link
Copy Markdown
Contributor

Add support for process/thread management functions in System.Diagnostics.Process for Haiku.

This is required to build managed runtime libraries for Haiku as well as running a simple "Hello, World!" application.

Part of #55803.

CopilotAI review requested due to automatic review settings November 21, 2025 15:01
@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Nov 21, 2025
@trungnt2910

Copy link
Copy Markdown
ContributorAuthor

C/c @am11

I just added the relevant parts of the common files to both, then we can rebase stuff later.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds Haiku operating system support to the System.Diagnostics.Process library, enabling process and thread management functionality for Haiku. This is part of the broader effort to support Haiku as a platform (#55803).

  • Implements Haiku-specific process/thread management APIs using native Haiku system calls
  • Adds proper platform attribute annotations for API surface compatibility
  • Integrates Haiku interop layer for process, thread, and image information

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
ProcessThread.csAdds [SupportedOSPlatform("haiku")] attributes to PriorityLevel property getter and setter
ProcessThread.Haiku.csImplements Haiku-specific thread priority management and processor time tracking using native BeOS-style priority values
ProcessManager.Haiku.csImplements process enumeration, module collection, and process information retrieval for Haiku using team and thread APIs
Process.csAdds [UnsupportedOSPlatform("haiku")] attributes to working set limit properties
Process.Haiku.csImplements Haiku-specific process properties including boot time caching, start time, processor time tracking, and platform-specific helper methods
System.Diagnostics.Process.csprojAdds Haiku target framework and includes Haiku-specific source files and interop definitions
Interop.OS.csDefines Haiku native interop for OS-level APIs including team/thread/area/system information structures and P/Invoke declarations
Interop.Libraries.csDefines the libroot library constant for Haiku native library imports
Interop.Image.csDefines Haiku native interop for image (module) information APIs

@@ -100,6 +101,7 @@ public ThreadPriorityLevel PriorityLevel
return _priorityLevel.Value;
}
[SupportedOSPlatform("windows")]

CopilotAINov 21, 2025

Copy link

Choose a reason for hiding this comment

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

The setter is missing [SupportedOSPlatform("linux")] and [SupportedOSPlatform("freebsd")] attributes. While these platforms throw PlatformNotSupportedException in their implementations, the attributes on the public API should match the getter's attributes to maintain consistency and accuracy in the API surface declarations. The presence of the getter attribute for these platforms indicates the API should be callable, even if it throws at runtime.

Suggested change
[SupportedOSPlatform("windows")]
[SupportedOSPlatform("windows")]
[SupportedOSPlatform("linux")]
[SupportedOSPlatform("freebsd")]

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Ignoring since it's not related to Haiku.

Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.OS.cs Outdated
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 3db3a8c to 822ecd1CompareNovember 21, 2025 15:28
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.Image.cs Outdated
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.Image.cs Outdated
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 822ecd1 to 7863babCompareNovember 21, 2025 15:51
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch 2 times, most recently from b7ee79c to f458cbdCompareNovember 22, 2025 02:53
@jeffhandleyjeffhandley added the needs-author-action An issue or pull request that requires more info or actions from the author. label Feb 1, 2026
@trungnt2910

Copy link
Copy Markdown
ContributorAuthor

Still tracking, will take action soon.

@dotnet-policy-servicedotnet-policy-serviceBot removed needs-author-action An issue or pull request that requires more info or actions from the author. no-recent-activity labels Feb 16, 2026
@jeffhandley

Copy link
Copy Markdown
Member

@trungnt2910 I pushed 0ee9ed3 to my fork; it merges main into your branch and resolves the conflict. Feel free to cherry-pick that commit if it's helpful.

@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from f458cbd to 5ed9cf0CompareApril 11, 2026 10:08
CopilotAI review requested due to automatic review settings April 11, 2026 10:08

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

Comment threadsrc/native/libs/configure.cmake
Comment threadsrc/native/libs/System.Native/pal_getosinfo.c
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.OS.cs
CopilotAI review requested due to automatic review settings April 11, 2026 11:23

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

Comment threadsrc/native/libs/System.Native/pal_getosinfo.c
Comment threadsrc/native/libs/configure.cmake
Comment on lines +14 to +21
public static void GetProcessInfos(ref ArrayBuilder<ProcessInfo> builder, string? processNameFilter)
{
int cookie = 0;
Interop.OS.TeamInfo info;

while ((Interop.OS.GetNextTeamInfo(ref cookie, out info)) == 0)
{
ProcessInfo? pi = GetProcessInfoFromTeamInfo(ref info, processNameFilter);

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

Interop.OS.TeamInfo is declared as an unsafe struct (due to the fixed buffer). Declaring/using it here (and calling GetNextTeamInfo) requires an unsafe context; as written this method will fail to compile for the Haiku TFM. Mark the method as unsafe (or refactor the interop type to avoid requiring unsafe at call sites).

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

Comment on lines +73 to +81
internal static ProcessInfo? CreateProcessInfo(int pid, string? processNameFilter = null)
{
// Negative PIDs aren't valid
ArgumentOutOfRangeException.ThrowIfNegative(pid);

Interop.OS.TeamInfo info;
int status = Interop.OS.GetTeamInfo(pid, out info);

if (status != 0)

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

CreateProcessInfo uses Interop.OS.TeamInfo (an unsafe struct because of the fixed buffer). This method needs to be in an unsafe context (e.g., mark it unsafe) or the interop needs to be reshaped so callers don't need unsafe; otherwise the Haiku build will fail to compile.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

Comment on lines +45 to +53
internal DateTime StartTimeCore
{
get
{
EnsureState(State.HaveNonExitedId);

Interop.OS.TeamInfo info;
int status = Interop.OS.GetTeamInfo(_processId, out info);

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

StartTimeCore uses Interop.OS.TeamInfo (an unsafe struct because of the fixed buffer). This getter must be in an unsafe context (e.g., mark the property/getter as unsafe) or the interop needs to be reshaped; otherwise the Haiku TFM will not compile.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

CopilotAI review requested due to automatic review settings April 11, 2026 11:53
@trungnt2910

trungnt2910 commented Apr 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Local builds are succeeding in both Release and Debug mode (with all my other open PRs on native components applied).

@jkotas
jkotas requested a review from adamsitnikApril 11, 2026 14:29
CopilotAI review requested due to automatic review settings April 11, 2026 14:43

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

CopilotAI review requested due to automatic review settings April 26, 2026 03:10

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

@am11

am11 commented Apr 26, 2026

Copy link
Copy Markdown
Member

LGTM!
cc @adamsitnik, PTAL.

trungnt2910and others added 3 commits April 26, 2026 22:46
Add support for process/thread management functions in
`System.Diagnostics.Process` for Haiku.
This is required to build managed runtime libraries for Haiku as well as
running a simple "Hello, World!" application.
Co-authored-by: Jessica Hamilton <jessica.l.hamilton@gmail.com>
Declare Haiku as a supported platform to MSBuild for all
`System.Diagnostics.Process` builds.
This prevents `CA1418` when using the `SupportedOSPlatform` attribute
with `haiku`.
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 50691e9 to d460910CompareApril 26, 2026 12:47
Otherwise `clang`-based builds will complain.
`clang` treats the `enum`s as unsigned `int`, triggering a sign conversion warning.
CopilotAI review requested due to automatic review settings April 26, 2026 13:18

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

@github-actions

github-actionsBot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
"version": 5,
"last_dispatched_commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "4e26169766106d54cf582a095878d6d439763335",
"last_reviewed_commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "4e26169766106d54cf582a095878d6d439763335",
"last_recorded_worker_run_id": "29674636225",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"review_id": 4730182665
}
]
}

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Holistic Review

Motivation: Justified. This adds System.Diagnostics.Process support for Haiku (part of #55803), which is a real prerequisite for building and running managed libraries on that platform. The approach follows the established per-platform partial-class pattern already used for iOS, SunOS, etc.

Approach: Sound and consistent with the codebase. New PInvokes are added as SystemNative_* entrypoints wrapped in #if HAVE_OS_H/HAVE_IMAGE_H with ENOTSUP fallbacks, structs are marshalled through PAL shims, and the managed side reuses ArrayBuilder/ProcessModuleCollection. Public API surface changes are additive [SupportedOSPlatform("haiku")] annotations on existing members, so no new API approval is required.

Summary: ⚠️ Needs Human Review. The change is well-structured and platform-isolated, so it cannot regress existing platforms. My main concern is the UTF-8 vs. ANSI string decoding of process/module names (flagged inline). There is also a dead GetNextTeamId PInvoke. Neither blocks the platform bring-up goal, but a maintainer familiar with the Haiku work should confirm the name-decoding intent.


Detailed Findings

⚠️ Correctness — Process/module name decoding uses the default code page, not UTF-8

See the inline comment on ProcessManager.Haiku.cs. new string((sbyte*)ptr, 0, length) interprets the bytes using the current ANSI code page, whereas Haiku names are UTF-8. Non-ASCII process/module names will be mangled. Decoding via Encoding.UTF8 (over the computed length) would match both the platform encoding and the convention used elsewhere for native UTF-8 strings.

💡 Dead code — SystemNative_GetNextTeamId appears unused

GetNextTeamId is declared in Interop.OS.cs, exported in entrypoints.c, and implemented in pal_getosinfo.c, but I found no managed caller (GetProcessInfos iterates via GetNextTeamInfo, and there is no EnumerateProcessIds/GetProcessIds for Haiku). If it is not needed for the current bring-up, consider removing it to avoid unused surface; if a follow-up will consume it, a brief note would help.

✅ Native PAL layer — guards and fallbacks look correct

Each entrypoint null-checks its out-params, memsets native structs before use, returns status_t codes directly, and provides (void)-cast ENOTSUP stubs when HAVE_OS_H/HAVE_IMAGE_H are absent. c_static_assert(SYSTEMNATIVE_MAX_PATH >= MAXPATHLEN) guards the image-name buffer. The struct field layouts match the managed [StructLayout(LayoutKind.Sequential)] definitions.

✅ Boot-time caching — race-safe

Process.Haiku.cs caches s_bootTimeTicks with Interlocked.Read/CompareExchange and reconciles with the winning writer's value, so concurrent initialization is handled correctly.

✅ Test coverage — no new tests, but consistent with platform bring-up

No unit tests are added, which is consistent with other platform-enablement PRs in this project where cross-platform System.Diagnostics.Process tests provide coverage once the platform is buildable. A human reviewer should confirm this matches the project's expectations for Haiku enablement.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 181.6 AIC · ⌖ 11 AIC · ⊞ 10K

private static unsafe string GetString(byte* ptr, int maxLength)
{
int length = new ReadOnlySpan<byte>(ptr, maxLength).IndexOf((byte)0);
return new string((sbyte*)ptr, 0, (length < 0) ? maxLength : length);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️new string((sbyte*)ptr, 0, length) decodes the bytes using the system default ANSI code page rather than UTF-8. Haiku uses UTF-8 for team and image names, so any process or module name containing non-ASCII characters will be misdecoded here. Consider Encoding.UTF8.GetString(new ReadOnlySpan<byte>(ptr, (length < 0) ? maxLength : length)) (you already compute the NUL-terminated length just above), which is the pattern used elsewhere in the codebase for UTF-8 native strings.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Diagnostics.Processcommunity-contributionIndicates that the PR has been added by a community memberos-haiku

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@trungnt2910@jeffhandley@am11@huoyaoyuan@jkotas
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Haiku: Process/thread management functions - #121883

Open
trungnt2910 wants to merge 4 commits into
dotnet:mainfrom
trungnt2910:dev/trungnt2910/haiku-lib-diagnostics
Open

Haiku: Process/thread management functions#121883
trungnt2910 wants to merge 4 commits into
dotnet:mainfrom
trungnt2910:dev/trungnt2910/haiku-lib-diagnostics

Conversation

@trungnt2910

Copy link
Copy Markdown
Contributor

Add support for process/thread management functions in System.Diagnostics.Process for Haiku.

This is required to build managed runtime libraries for Haiku as well as running a simple "Hello, World!" application.

Part of #55803.

CopilotAI review requested due to automatic review settings November 21, 2025 15:01
@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Nov 21, 2025
@trungnt2910

Copy link
Copy Markdown
ContributorAuthor

C/c @am11

I just added the relevant parts of the common files to both, then we can rebase stuff later.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds Haiku operating system support to the System.Diagnostics.Process library, enabling process and thread management functionality for Haiku. This is part of the broader effort to support Haiku as a platform (#55803).

  • Implements Haiku-specific process/thread management APIs using native Haiku system calls
  • Adds proper platform attribute annotations for API surface compatibility
  • Integrates Haiku interop layer for process, thread, and image information

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
ProcessThread.csAdds [SupportedOSPlatform("haiku")] attributes to PriorityLevel property getter and setter
ProcessThread.Haiku.csImplements Haiku-specific thread priority management and processor time tracking using native BeOS-style priority values
ProcessManager.Haiku.csImplements process enumeration, module collection, and process information retrieval for Haiku using team and thread APIs
Process.csAdds [UnsupportedOSPlatform("haiku")] attributes to working set limit properties
Process.Haiku.csImplements Haiku-specific process properties including boot time caching, start time, processor time tracking, and platform-specific helper methods
System.Diagnostics.Process.csprojAdds Haiku target framework and includes Haiku-specific source files and interop definitions
Interop.OS.csDefines Haiku native interop for OS-level APIs including team/thread/area/system information structures and P/Invoke declarations
Interop.Libraries.csDefines the libroot library constant for Haiku native library imports
Interop.Image.csDefines Haiku native interop for image (module) information APIs

@@ -100,6 +101,7 @@ public ThreadPriorityLevel PriorityLevel
return _priorityLevel.Value;
}
[SupportedOSPlatform("windows")]

CopilotAINov 21, 2025

Copy link

Choose a reason for hiding this comment

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

The setter is missing [SupportedOSPlatform("linux")] and [SupportedOSPlatform("freebsd")] attributes. While these platforms throw PlatformNotSupportedException in their implementations, the attributes on the public API should match the getter's attributes to maintain consistency and accuracy in the API surface declarations. The presence of the getter attribute for these platforms indicates the API should be callable, even if it throws at runtime.

Suggested change
[SupportedOSPlatform("windows")]
[SupportedOSPlatform("windows")]
[SupportedOSPlatform("linux")]
[SupportedOSPlatform("freebsd")]

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Ignoring since it's not related to Haiku.

Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.OS.cs Outdated
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 3db3a8c to 822ecd1CompareNovember 21, 2025 15:28
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.Image.cs Outdated
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.Image.cs Outdated
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 822ecd1 to 7863babCompareNovember 21, 2025 15:51
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch 2 times, most recently from b7ee79c to f458cbdCompareNovember 22, 2025 02:53
@jeffhandleyjeffhandley added the needs-author-action An issue or pull request that requires more info or actions from the author. label Feb 1, 2026
@trungnt2910

Copy link
Copy Markdown
ContributorAuthor

Still tracking, will take action soon.

@dotnet-policy-servicedotnet-policy-serviceBot removed needs-author-action An issue or pull request that requires more info or actions from the author. no-recent-activity labels Feb 16, 2026
@jeffhandley

Copy link
Copy Markdown
Member

@trungnt2910 I pushed 0ee9ed3 to my fork; it merges main into your branch and resolves the conflict. Feel free to cherry-pick that commit if it's helpful.

@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from f458cbd to 5ed9cf0CompareApril 11, 2026 10:08
CopilotAI review requested due to automatic review settings April 11, 2026 10:08

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

Comment threadsrc/native/libs/configure.cmake
Comment threadsrc/native/libs/System.Native/pal_getosinfo.c
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.OS.cs
CopilotAI review requested due to automatic review settings April 11, 2026 11:23

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

Comment threadsrc/native/libs/System.Native/pal_getosinfo.c
Comment threadsrc/native/libs/configure.cmake
Comment on lines +14 to +21
public static void GetProcessInfos(ref ArrayBuilder<ProcessInfo> builder, string? processNameFilter)
{
int cookie = 0;
Interop.OS.TeamInfo info;

while ((Interop.OS.GetNextTeamInfo(ref cookie, out info)) == 0)
{
ProcessInfo? pi = GetProcessInfoFromTeamInfo(ref info, processNameFilter);

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

Interop.OS.TeamInfo is declared as an unsafe struct (due to the fixed buffer). Declaring/using it here (and calling GetNextTeamInfo) requires an unsafe context; as written this method will fail to compile for the Haiku TFM. Mark the method as unsafe (or refactor the interop type to avoid requiring unsafe at call sites).

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

Comment on lines +73 to +81
internal static ProcessInfo? CreateProcessInfo(int pid, string? processNameFilter = null)
{
// Negative PIDs aren't valid
ArgumentOutOfRangeException.ThrowIfNegative(pid);

Interop.OS.TeamInfo info;
int status = Interop.OS.GetTeamInfo(pid, out info);

if (status != 0)

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

CreateProcessInfo uses Interop.OS.TeamInfo (an unsafe struct because of the fixed buffer). This method needs to be in an unsafe context (e.g., mark it unsafe) or the interop needs to be reshaped so callers don't need unsafe; otherwise the Haiku build will fail to compile.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

Comment on lines +45 to +53
internal DateTime StartTimeCore
{
get
{
EnsureState(State.HaveNonExitedId);

Interop.OS.TeamInfo info;
int status = Interop.OS.GetTeamInfo(_processId, out info);

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

StartTimeCore uses Interop.OS.TeamInfo (an unsafe struct because of the fixed buffer). This getter must be in an unsafe context (e.g., mark the property/getter as unsafe) or the interop needs to be reshaped; otherwise the Haiku TFM will not compile.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

CopilotAI review requested due to automatic review settings April 11, 2026 11:53
@trungnt2910

trungnt2910 commented Apr 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Local builds are succeeding in both Release and Debug mode (with all my other open PRs on native components applied).

@jkotas
jkotas requested a review from adamsitnikApril 11, 2026 14:29
CopilotAI review requested due to automatic review settings April 11, 2026 14:43

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

CopilotAI review requested due to automatic review settings April 26, 2026 03:10

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

@am11

am11 commented Apr 26, 2026

Copy link
Copy Markdown
Member

LGTM!
cc @adamsitnik, PTAL.

trungnt2910and others added 3 commits April 26, 2026 22:46
Add support for process/thread management functions in
`System.Diagnostics.Process` for Haiku.
This is required to build managed runtime libraries for Haiku as well as
running a simple "Hello, World!" application.
Co-authored-by: Jessica Hamilton <jessica.l.hamilton@gmail.com>
Declare Haiku as a supported platform to MSBuild for all
`System.Diagnostics.Process` builds.
This prevents `CA1418` when using the `SupportedOSPlatform` attribute
with `haiku`.
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 50691e9 to d460910CompareApril 26, 2026 12:47
Otherwise `clang`-based builds will complain.
`clang` treats the `enum`s as unsigned `int`, triggering a sign conversion warning.
CopilotAI review requested due to automatic review settings April 26, 2026 13:18

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

@github-actions

github-actionsBot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
"version": 5,
"last_dispatched_commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "4e26169766106d54cf582a095878d6d439763335",
"last_reviewed_commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "4e26169766106d54cf582a095878d6d439763335",
"last_recorded_worker_run_id": "29674636225",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"review_id": 4730182665
}
]
}

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Holistic Review

Motivation: Justified. This adds System.Diagnostics.Process support for Haiku (part of #55803), which is a real prerequisite for building and running managed libraries on that platform. The approach follows the established per-platform partial-class pattern already used for iOS, SunOS, etc.

Approach: Sound and consistent with the codebase. New PInvokes are added as SystemNative_* entrypoints wrapped in #if HAVE_OS_H/HAVE_IMAGE_H with ENOTSUP fallbacks, structs are marshalled through PAL shims, and the managed side reuses ArrayBuilder/ProcessModuleCollection. Public API surface changes are additive [SupportedOSPlatform("haiku")] annotations on existing members, so no new API approval is required.

Summary: ⚠️ Needs Human Review. The change is well-structured and platform-isolated, so it cannot regress existing platforms. My main concern is the UTF-8 vs. ANSI string decoding of process/module names (flagged inline). There is also a dead GetNextTeamId PInvoke. Neither blocks the platform bring-up goal, but a maintainer familiar with the Haiku work should confirm the name-decoding intent.


Detailed Findings

⚠️ Correctness — Process/module name decoding uses the default code page, not UTF-8

See the inline comment on ProcessManager.Haiku.cs. new string((sbyte*)ptr, 0, length) interprets the bytes using the current ANSI code page, whereas Haiku names are UTF-8. Non-ASCII process/module names will be mangled. Decoding via Encoding.UTF8 (over the computed length) would match both the platform encoding and the convention used elsewhere for native UTF-8 strings.

💡 Dead code — SystemNative_GetNextTeamId appears unused

GetNextTeamId is declared in Interop.OS.cs, exported in entrypoints.c, and implemented in pal_getosinfo.c, but I found no managed caller (GetProcessInfos iterates via GetNextTeamInfo, and there is no EnumerateProcessIds/GetProcessIds for Haiku). If it is not needed for the current bring-up, consider removing it to avoid unused surface; if a follow-up will consume it, a brief note would help.

✅ Native PAL layer — guards and fallbacks look correct

Each entrypoint null-checks its out-params, memsets native structs before use, returns status_t codes directly, and provides (void)-cast ENOTSUP stubs when HAVE_OS_H/HAVE_IMAGE_H are absent. c_static_assert(SYSTEMNATIVE_MAX_PATH >= MAXPATHLEN) guards the image-name buffer. The struct field layouts match the managed [StructLayout(LayoutKind.Sequential)] definitions.

✅ Boot-time caching — race-safe

Process.Haiku.cs caches s_bootTimeTicks with Interlocked.Read/CompareExchange and reconciles with the winning writer's value, so concurrent initialization is handled correctly.

✅ Test coverage — no new tests, but consistent with platform bring-up

No unit tests are added, which is consistent with other platform-enablement PRs in this project where cross-platform System.Diagnostics.Process tests provide coverage once the platform is buildable. A human reviewer should confirm this matches the project's expectations for Haiku enablement.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 181.6 AIC · ⌖ 11 AIC · ⊞ 10K

private static unsafe string GetString(byte* ptr, int maxLength)
{
int length = new ReadOnlySpan<byte>(ptr, maxLength).IndexOf((byte)0);
return new string((sbyte*)ptr, 0, (length < 0) ? maxLength : length);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️new string((sbyte*)ptr, 0, length) decodes the bytes using the system default ANSI code page rather than UTF-8. Haiku uses UTF-8 for team and image names, so any process or module name containing non-ASCII characters will be misdecoded here. Consider Encoding.UTF8.GetString(new ReadOnlySpan<byte>(ptr, (length < 0) ? maxLength : length)) (you already compute the NUL-terminated length just above), which is the pattern used elsewhere in the codebase for UTF-8 native strings.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Diagnostics.Processcommunity-contributionIndicates that the PR has been added by a community memberos-haiku

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@trungnt2910@jeffhandley@am11@huoyaoyuan@jkotas
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Haiku: Process/thread management functions - #121883

Open
trungnt2910 wants to merge 4 commits into
dotnet:mainfrom
trungnt2910:dev/trungnt2910/haiku-lib-diagnostics
Open

Haiku: Process/thread management functions#121883
trungnt2910 wants to merge 4 commits into
dotnet:mainfrom
trungnt2910:dev/trungnt2910/haiku-lib-diagnostics

Conversation

@trungnt2910

Copy link
Copy Markdown
Contributor

Add support for process/thread management functions in System.Diagnostics.Process for Haiku.

This is required to build managed runtime libraries for Haiku as well as running a simple "Hello, World!" application.

Part of #55803.

CopilotAI review requested due to automatic review settings November 21, 2025 15:01
@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Nov 21, 2025
@trungnt2910

Copy link
Copy Markdown
ContributorAuthor

C/c @am11

I just added the relevant parts of the common files to both, then we can rebase stuff later.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds Haiku operating system support to the System.Diagnostics.Process library, enabling process and thread management functionality for Haiku. This is part of the broader effort to support Haiku as a platform (#55803).

  • Implements Haiku-specific process/thread management APIs using native Haiku system calls
  • Adds proper platform attribute annotations for API surface compatibility
  • Integrates Haiku interop layer for process, thread, and image information

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
ProcessThread.csAdds [SupportedOSPlatform("haiku")] attributes to PriorityLevel property getter and setter
ProcessThread.Haiku.csImplements Haiku-specific thread priority management and processor time tracking using native BeOS-style priority values
ProcessManager.Haiku.csImplements process enumeration, module collection, and process information retrieval for Haiku using team and thread APIs
Process.csAdds [UnsupportedOSPlatform("haiku")] attributes to working set limit properties
Process.Haiku.csImplements Haiku-specific process properties including boot time caching, start time, processor time tracking, and platform-specific helper methods
System.Diagnostics.Process.csprojAdds Haiku target framework and includes Haiku-specific source files and interop definitions
Interop.OS.csDefines Haiku native interop for OS-level APIs including team/thread/area/system information structures and P/Invoke declarations
Interop.Libraries.csDefines the libroot library constant for Haiku native library imports
Interop.Image.csDefines Haiku native interop for image (module) information APIs

@@ -100,6 +101,7 @@ public ThreadPriorityLevel PriorityLevel
return _priorityLevel.Value;
}
[SupportedOSPlatform("windows")]

CopilotAINov 21, 2025

Copy link

Choose a reason for hiding this comment

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

The setter is missing [SupportedOSPlatform("linux")] and [SupportedOSPlatform("freebsd")] attributes. While these platforms throw PlatformNotSupportedException in their implementations, the attributes on the public API should match the getter's attributes to maintain consistency and accuracy in the API surface declarations. The presence of the getter attribute for these platforms indicates the API should be callable, even if it throws at runtime.

Suggested change
[SupportedOSPlatform("windows")]
[SupportedOSPlatform("windows")]
[SupportedOSPlatform("linux")]
[SupportedOSPlatform("freebsd")]

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Ignoring since it's not related to Haiku.

Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.OS.cs Outdated
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 3db3a8c to 822ecd1CompareNovember 21, 2025 15:28
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.Image.cs Outdated
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.Image.cs Outdated
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 822ecd1 to 7863babCompareNovember 21, 2025 15:51
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch 2 times, most recently from b7ee79c to f458cbdCompareNovember 22, 2025 02:53
@jeffhandleyjeffhandley added the needs-author-action An issue or pull request that requires more info or actions from the author. label Feb 1, 2026
@trungnt2910

Copy link
Copy Markdown
ContributorAuthor

Still tracking, will take action soon.

@dotnet-policy-servicedotnet-policy-serviceBot removed needs-author-action An issue or pull request that requires more info or actions from the author. no-recent-activity labels Feb 16, 2026
@jeffhandley

Copy link
Copy Markdown
Member

@trungnt2910 I pushed 0ee9ed3 to my fork; it merges main into your branch and resolves the conflict. Feel free to cherry-pick that commit if it's helpful.

@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from f458cbd to 5ed9cf0CompareApril 11, 2026 10:08
CopilotAI review requested due to automatic review settings April 11, 2026 10:08

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

Comment threadsrc/native/libs/configure.cmake
Comment threadsrc/native/libs/System.Native/pal_getosinfo.c
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.OS.cs
CopilotAI review requested due to automatic review settings April 11, 2026 11:23

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

Comment threadsrc/native/libs/System.Native/pal_getosinfo.c
Comment threadsrc/native/libs/configure.cmake
Comment on lines +14 to +21
public static void GetProcessInfos(ref ArrayBuilder<ProcessInfo> builder, string? processNameFilter)
{
int cookie = 0;
Interop.OS.TeamInfo info;

while ((Interop.OS.GetNextTeamInfo(ref cookie, out info)) == 0)
{
ProcessInfo? pi = GetProcessInfoFromTeamInfo(ref info, processNameFilter);

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

Interop.OS.TeamInfo is declared as an unsafe struct (due to the fixed buffer). Declaring/using it here (and calling GetNextTeamInfo) requires an unsafe context; as written this method will fail to compile for the Haiku TFM. Mark the method as unsafe (or refactor the interop type to avoid requiring unsafe at call sites).

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

Comment on lines +73 to +81
internal static ProcessInfo? CreateProcessInfo(int pid, string? processNameFilter = null)
{
// Negative PIDs aren't valid
ArgumentOutOfRangeException.ThrowIfNegative(pid);

Interop.OS.TeamInfo info;
int status = Interop.OS.GetTeamInfo(pid, out info);

if (status != 0)

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

CreateProcessInfo uses Interop.OS.TeamInfo (an unsafe struct because of the fixed buffer). This method needs to be in an unsafe context (e.g., mark it unsafe) or the interop needs to be reshaped so callers don't need unsafe; otherwise the Haiku build will fail to compile.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

Comment on lines +45 to +53
internal DateTime StartTimeCore
{
get
{
EnsureState(State.HaveNonExitedId);

Interop.OS.TeamInfo info;
int status = Interop.OS.GetTeamInfo(_processId, out info);

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

StartTimeCore uses Interop.OS.TeamInfo (an unsafe struct because of the fixed buffer). This getter must be in an unsafe context (e.g., mark the property/getter as unsafe) or the interop needs to be reshaped; otherwise the Haiku TFM will not compile.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

CopilotAI review requested due to automatic review settings April 11, 2026 11:53
@trungnt2910

trungnt2910 commented Apr 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Local builds are succeeding in both Release and Debug mode (with all my other open PRs on native components applied).

@jkotas
jkotas requested a review from adamsitnikApril 11, 2026 14:29
CopilotAI review requested due to automatic review settings April 11, 2026 14:43

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

CopilotAI review requested due to automatic review settings April 26, 2026 03:10

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

@am11

am11 commented Apr 26, 2026

Copy link
Copy Markdown
Member

LGTM!
cc @adamsitnik, PTAL.

trungnt2910and others added 3 commits April 26, 2026 22:46
Add support for process/thread management functions in
`System.Diagnostics.Process` for Haiku.
This is required to build managed runtime libraries for Haiku as well as
running a simple "Hello, World!" application.
Co-authored-by: Jessica Hamilton <jessica.l.hamilton@gmail.com>
Declare Haiku as a supported platform to MSBuild for all
`System.Diagnostics.Process` builds.
This prevents `CA1418` when using the `SupportedOSPlatform` attribute
with `haiku`.
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 50691e9 to d460910CompareApril 26, 2026 12:47
Otherwise `clang`-based builds will complain.
`clang` treats the `enum`s as unsigned `int`, triggering a sign conversion warning.
CopilotAI review requested due to automatic review settings April 26, 2026 13:18

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

@github-actions

github-actionsBot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
"version": 5,
"last_dispatched_commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "4e26169766106d54cf582a095878d6d439763335",
"last_reviewed_commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "4e26169766106d54cf582a095878d6d439763335",
"last_recorded_worker_run_id": "29674636225",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"review_id": 4730182665
}
]
}

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Holistic Review

Motivation: Justified. This adds System.Diagnostics.Process support for Haiku (part of #55803), which is a real prerequisite for building and running managed libraries on that platform. The approach follows the established per-platform partial-class pattern already used for iOS, SunOS, etc.

Approach: Sound and consistent with the codebase. New PInvokes are added as SystemNative_* entrypoints wrapped in #if HAVE_OS_H/HAVE_IMAGE_H with ENOTSUP fallbacks, structs are marshalled through PAL shims, and the managed side reuses ArrayBuilder/ProcessModuleCollection. Public API surface changes are additive [SupportedOSPlatform("haiku")] annotations on existing members, so no new API approval is required.

Summary: ⚠️ Needs Human Review. The change is well-structured and platform-isolated, so it cannot regress existing platforms. My main concern is the UTF-8 vs. ANSI string decoding of process/module names (flagged inline). There is also a dead GetNextTeamId PInvoke. Neither blocks the platform bring-up goal, but a maintainer familiar with the Haiku work should confirm the name-decoding intent.


Detailed Findings

⚠️ Correctness — Process/module name decoding uses the default code page, not UTF-8

See the inline comment on ProcessManager.Haiku.cs. new string((sbyte*)ptr, 0, length) interprets the bytes using the current ANSI code page, whereas Haiku names are UTF-8. Non-ASCII process/module names will be mangled. Decoding via Encoding.UTF8 (over the computed length) would match both the platform encoding and the convention used elsewhere for native UTF-8 strings.

💡 Dead code — SystemNative_GetNextTeamId appears unused

GetNextTeamId is declared in Interop.OS.cs, exported in entrypoints.c, and implemented in pal_getosinfo.c, but I found no managed caller (GetProcessInfos iterates via GetNextTeamInfo, and there is no EnumerateProcessIds/GetProcessIds for Haiku). If it is not needed for the current bring-up, consider removing it to avoid unused surface; if a follow-up will consume it, a brief note would help.

✅ Native PAL layer — guards and fallbacks look correct

Each entrypoint null-checks its out-params, memsets native structs before use, returns status_t codes directly, and provides (void)-cast ENOTSUP stubs when HAVE_OS_H/HAVE_IMAGE_H are absent. c_static_assert(SYSTEMNATIVE_MAX_PATH >= MAXPATHLEN) guards the image-name buffer. The struct field layouts match the managed [StructLayout(LayoutKind.Sequential)] definitions.

✅ Boot-time caching — race-safe

Process.Haiku.cs caches s_bootTimeTicks with Interlocked.Read/CompareExchange and reconciles with the winning writer's value, so concurrent initialization is handled correctly.

✅ Test coverage — no new tests, but consistent with platform bring-up

No unit tests are added, which is consistent with other platform-enablement PRs in this project where cross-platform System.Diagnostics.Process tests provide coverage once the platform is buildable. A human reviewer should confirm this matches the project's expectations for Haiku enablement.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 181.6 AIC · ⌖ 11 AIC · ⊞ 10K

private static unsafe string GetString(byte* ptr, int maxLength)
{
int length = new ReadOnlySpan<byte>(ptr, maxLength).IndexOf((byte)0);
return new string((sbyte*)ptr, 0, (length < 0) ? maxLength : length);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️new string((sbyte*)ptr, 0, length) decodes the bytes using the system default ANSI code page rather than UTF-8. Haiku uses UTF-8 for team and image names, so any process or module name containing non-ASCII characters will be misdecoded here. Consider Encoding.UTF8.GetString(new ReadOnlySpan<byte>(ptr, (length < 0) ? maxLength : length)) (you already compute the NUL-terminated length just above), which is the pattern used elsewhere in the codebase for UTF-8 native strings.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Diagnostics.Processcommunity-contributionIndicates that the PR has been added by a community memberos-haiku

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@trungnt2910@jeffhandley@am11@huoyaoyuan@jkotas
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Haiku: Process/thread management functions - #121883

Open
trungnt2910 wants to merge 4 commits into
dotnet:mainfrom
trungnt2910:dev/trungnt2910/haiku-lib-diagnostics
Open

Haiku: Process/thread management functions#121883
trungnt2910 wants to merge 4 commits into
dotnet:mainfrom
trungnt2910:dev/trungnt2910/haiku-lib-diagnostics

Conversation

@trungnt2910

Copy link
Copy Markdown
Contributor

Add support for process/thread management functions in System.Diagnostics.Process for Haiku.

This is required to build managed runtime libraries for Haiku as well as running a simple "Hello, World!" application.

Part of #55803.

CopilotAI review requested due to automatic review settings November 21, 2025 15:01
@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Nov 21, 2025
@trungnt2910

Copy link
Copy Markdown
ContributorAuthor

C/c @am11

I just added the relevant parts of the common files to both, then we can rebase stuff later.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds Haiku operating system support to the System.Diagnostics.Process library, enabling process and thread management functionality for Haiku. This is part of the broader effort to support Haiku as a platform (#55803).

  • Implements Haiku-specific process/thread management APIs using native Haiku system calls
  • Adds proper platform attribute annotations for API surface compatibility
  • Integrates Haiku interop layer for process, thread, and image information

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
ProcessThread.csAdds [SupportedOSPlatform("haiku")] attributes to PriorityLevel property getter and setter
ProcessThread.Haiku.csImplements Haiku-specific thread priority management and processor time tracking using native BeOS-style priority values
ProcessManager.Haiku.csImplements process enumeration, module collection, and process information retrieval for Haiku using team and thread APIs
Process.csAdds [UnsupportedOSPlatform("haiku")] attributes to working set limit properties
Process.Haiku.csImplements Haiku-specific process properties including boot time caching, start time, processor time tracking, and platform-specific helper methods
System.Diagnostics.Process.csprojAdds Haiku target framework and includes Haiku-specific source files and interop definitions
Interop.OS.csDefines Haiku native interop for OS-level APIs including team/thread/area/system information structures and P/Invoke declarations
Interop.Libraries.csDefines the libroot library constant for Haiku native library imports
Interop.Image.csDefines Haiku native interop for image (module) information APIs

@@ -100,6 +101,7 @@ public ThreadPriorityLevel PriorityLevel
return _priorityLevel.Value;
}
[SupportedOSPlatform("windows")]

CopilotAINov 21, 2025

Copy link

Choose a reason for hiding this comment

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

The setter is missing [SupportedOSPlatform("linux")] and [SupportedOSPlatform("freebsd")] attributes. While these platforms throw PlatformNotSupportedException in their implementations, the attributes on the public API should match the getter's attributes to maintain consistency and accuracy in the API surface declarations. The presence of the getter attribute for these platforms indicates the API should be callable, even if it throws at runtime.

Suggested change
[SupportedOSPlatform("windows")]
[SupportedOSPlatform("windows")]
[SupportedOSPlatform("linux")]
[SupportedOSPlatform("freebsd")]

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Ignoring since it's not related to Haiku.

Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.OS.cs Outdated
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 3db3a8c to 822ecd1CompareNovember 21, 2025 15:28
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.Image.cs Outdated
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.Image.cs Outdated
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 822ecd1 to 7863babCompareNovember 21, 2025 15:51
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch 2 times, most recently from b7ee79c to f458cbdCompareNovember 22, 2025 02:53
@jeffhandleyjeffhandley added the needs-author-action An issue or pull request that requires more info or actions from the author. label Feb 1, 2026
@trungnt2910

Copy link
Copy Markdown
ContributorAuthor

Still tracking, will take action soon.

@dotnet-policy-servicedotnet-policy-serviceBot removed needs-author-action An issue or pull request that requires more info or actions from the author. no-recent-activity labels Feb 16, 2026
@jeffhandley

Copy link
Copy Markdown
Member

@trungnt2910 I pushed 0ee9ed3 to my fork; it merges main into your branch and resolves the conflict. Feel free to cherry-pick that commit if it's helpful.

@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from f458cbd to 5ed9cf0CompareApril 11, 2026 10:08
CopilotAI review requested due to automatic review settings April 11, 2026 10:08

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

Comment threadsrc/native/libs/configure.cmake
Comment threadsrc/native/libs/System.Native/pal_getosinfo.c
Comment threadsrc/libraries/Common/src/Interop/Haiku/Interop.OS.cs
CopilotAI review requested due to automatic review settings April 11, 2026 11:23

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

Comment threadsrc/native/libs/System.Native/pal_getosinfo.c
Comment threadsrc/native/libs/configure.cmake
Comment on lines +14 to +21
public static void GetProcessInfos(ref ArrayBuilder<ProcessInfo> builder, string? processNameFilter)
{
int cookie = 0;
Interop.OS.TeamInfo info;

while ((Interop.OS.GetNextTeamInfo(ref cookie, out info)) == 0)
{
ProcessInfo? pi = GetProcessInfoFromTeamInfo(ref info, processNameFilter);

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

Interop.OS.TeamInfo is declared as an unsafe struct (due to the fixed buffer). Declaring/using it here (and calling GetNextTeamInfo) requires an unsafe context; as written this method will fail to compile for the Haiku TFM. Mark the method as unsafe (or refactor the interop type to avoid requiring unsafe at call sites).

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

Comment on lines +73 to +81
internal static ProcessInfo? CreateProcessInfo(int pid, string? processNameFilter = null)
{
// Negative PIDs aren't valid
ArgumentOutOfRangeException.ThrowIfNegative(pid);

Interop.OS.TeamInfo info;
int status = Interop.OS.GetTeamInfo(pid, out info);

if (status != 0)

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

CreateProcessInfo uses Interop.OS.TeamInfo (an unsafe struct because of the fixed buffer). This method needs to be in an unsafe context (e.g., mark it unsafe) or the interop needs to be reshaped so callers don't need unsafe; otherwise the Haiku build will fail to compile.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

Comment on lines +45 to +53
internal DateTime StartTimeCore
{
get
{
EnsureState(State.HaveNonExitedId);

Interop.OS.TeamInfo info;
int status = Interop.OS.GetTeamInfo(_processId, out info);

CopilotAIApr 11, 2026

Copy link

Choose a reason for hiding this comment

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

StartTimeCore uses Interop.OS.TeamInfo (an unsafe struct because of the fixed buffer). This getter must be in an unsafe context (e.g., mark the property/getter as unsafe) or the interop needs to be reshaped; otherwise the Haiku TFM will not compile.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Haven't seen any build errors so far.

CopilotAI review requested due to automatic review settings April 11, 2026 11:53
@trungnt2910

trungnt2910 commented Apr 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Local builds are succeeding in both Release and Debug mode (with all my other open PRs on native components applied).

@jkotas
jkotas requested a review from adamsitnikApril 11, 2026 14:29
CopilotAI review requested due to automatic review settings April 11, 2026 14:43

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

CopilotAI review requested due to automatic review settings April 26, 2026 03:10

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

@am11

am11 commented Apr 26, 2026

Copy link
Copy Markdown
Member

LGTM!
cc @adamsitnik, PTAL.

trungnt2910and others added 3 commits April 26, 2026 22:46
Add support for process/thread management functions in
`System.Diagnostics.Process` for Haiku.
This is required to build managed runtime libraries for Haiku as well as
running a simple "Hello, World!" application.
Co-authored-by: Jessica Hamilton <jessica.l.hamilton@gmail.com>
Declare Haiku as a supported platform to MSBuild for all
`System.Diagnostics.Process` builds.
This prevents `CA1418` when using the `SupportedOSPlatform` attribute
with `haiku`.
@trungnt2910
trungnt2910force-pushed the dev/trungnt2910/haiku-lib-diagnostics branch from 50691e9 to d460910CompareApril 26, 2026 12:47
Otherwise `clang`-based builds will complain.
`clang` treats the `enum`s as unsigned `int`, triggering a sign conversion warning.
CopilotAI review requested due to automatic review settings April 26, 2026 13:18

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

@github-actions

github-actionsBot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
"version": 5,
"last_dispatched_commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "4e26169766106d54cf582a095878d6d439763335",
"last_reviewed_commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "4e26169766106d54cf582a095878d6d439763335",
"last_recorded_worker_run_id": "29674636225",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "0b80a9e38506d4f0915adadda63ae6e315a229cb",
"review_id": 4730182665
}
]
}

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Holistic Review

Motivation: Justified. This adds System.Diagnostics.Process support for Haiku (part of #55803), which is a real prerequisite for building and running managed libraries on that platform. The approach follows the established per-platform partial-class pattern already used for iOS, SunOS, etc.

Approach: Sound and consistent with the codebase. New PInvokes are added as SystemNative_* entrypoints wrapped in #if HAVE_OS_H/HAVE_IMAGE_H with ENOTSUP fallbacks, structs are marshalled through PAL shims, and the managed side reuses ArrayBuilder/ProcessModuleCollection. Public API surface changes are additive [SupportedOSPlatform("haiku")] annotations on existing members, so no new API approval is required.

Summary: ⚠️ Needs Human Review. The change is well-structured and platform-isolated, so it cannot regress existing platforms. My main concern is the UTF-8 vs. ANSI string decoding of process/module names (flagged inline). There is also a dead GetNextTeamId PInvoke. Neither blocks the platform bring-up goal, but a maintainer familiar with the Haiku work should confirm the name-decoding intent.


Detailed Findings

⚠️ Correctness — Process/module name decoding uses the default code page, not UTF-8

See the inline comment on ProcessManager.Haiku.cs. new string((sbyte*)ptr, 0, length) interprets the bytes using the current ANSI code page, whereas Haiku names are UTF-8. Non-ASCII process/module names will be mangled. Decoding via Encoding.UTF8 (over the computed length) would match both the platform encoding and the convention used elsewhere for native UTF-8 strings.

💡 Dead code — SystemNative_GetNextTeamId appears unused

GetNextTeamId is declared in Interop.OS.cs, exported in entrypoints.c, and implemented in pal_getosinfo.c, but I found no managed caller (GetProcessInfos iterates via GetNextTeamInfo, and there is no EnumerateProcessIds/GetProcessIds for Haiku). If it is not needed for the current bring-up, consider removing it to avoid unused surface; if a follow-up will consume it, a brief note would help.

✅ Native PAL layer — guards and fallbacks look correct

Each entrypoint null-checks its out-params, memsets native structs before use, returns status_t codes directly, and provides (void)-cast ENOTSUP stubs when HAVE_OS_H/HAVE_IMAGE_H are absent. c_static_assert(SYSTEMNATIVE_MAX_PATH >= MAXPATHLEN) guards the image-name buffer. The struct field layouts match the managed [StructLayout(LayoutKind.Sequential)] definitions.

✅ Boot-time caching — race-safe

Process.Haiku.cs caches s_bootTimeTicks with Interlocked.Read/CompareExchange and reconciles with the winning writer's value, so concurrent initialization is handled correctly.

✅ Test coverage — no new tests, but consistent with platform bring-up

No unit tests are added, which is consistent with other platform-enablement PRs in this project where cross-platform System.Diagnostics.Process tests provide coverage once the platform is buildable. A human reviewer should confirm this matches the project's expectations for Haiku enablement.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 181.6 AIC · ⌖ 11 AIC · ⊞ 10K

private static unsafe string GetString(byte* ptr, int maxLength)
{
int length = new ReadOnlySpan<byte>(ptr, maxLength).IndexOf((byte)0);
return new string((sbyte*)ptr, 0, (length < 0) ? maxLength : length);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️new string((sbyte*)ptr, 0, length) decodes the bytes using the system default ANSI code page rather than UTF-8. Haiku uses UTF-8 for team and image names, so any process or module name containing non-ASCII characters will be misdecoded here. Consider Encoding.UTF8.GetString(new ReadOnlySpan<byte>(ptr, (length < 0) ? maxLength : length)) (you already compute the NUL-terminated length just above), which is the pattern used elsewhere in the codebase for UTF-8 native strings.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Diagnostics.Processcommunity-contributionIndicates that the PR has been added by a community memberos-haiku

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@trungnt2910@jeffhandley@am11@huoyaoyuan@jkotas