mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-17 05:11:13 +08:00
Fix CI env-file tearing and capture Linux native crash dumps (#4517)
Two unrelated CI faults in the GitHub Actions build/test matrix. ## Build steps fail after a successful compile — fixes #4458 `version.json` sets `cloudBuild.setVersionVariables`, so Nerdbank.GitVersioning's `SetCloudBuildVersionVars` target runs **once per project**, and on GitHub Actions the provider implements "set a variable" as an append to the file named by `$GITHUB_ENV`. The `Build` step is deliberately parallel across MSBuild nodes, so several nodes append to that one file concurrently, a line tears, and the runner rejects the fragment after the compile has already succeeded. Reproduced locally — one `GetBuildVersion` with `GITHUB_ACTIONS=true` appends three lines per project: ``` GitAssemblyInformationalVersion=2.0.337-preview+138086863b GitBuildVersion=2.0.337.4992 GitBuildVersionSimple=2.0.337 ``` A stray `7` is exactly what a torn `GitBuildVersion=2.0.3x7.yyyyy` leaves behind. After this change the same command writes **0 bytes**, and `Version`, `AssemblyVersion`, `FileVersion` and `PackageVersion` are unchanged (`2.0.337-preview.g138086863b`) — verified on both a sample project and `Opc.Ua.Sessions.Tests`. Nothing reads those three variables. Every workflow and pipeline that consumes version information runs `.azurepipelines/set-version.ps1` first, and `nbgv cloud -c -a` there writes the same `Git*` variables plus the `NBGV_*` set that `version.props` and `common.props` actually use — once, from a single process. This leaves that script as the only writer. Two notes against the fixes suggested in the issue: - `NerdbankGitVersioningSetCloudBuildVariables` does not exist in NBGV 3.10.94. The real gate is `'$(NBGV_SetCloudBuildVersionVars)' != 'false'`, so the suggested `/p:` would have been silently ignored. - Setting `cloudBuild.setVersionVariables: false` **would regress every version number**. The version height is the commit count since `version.json` last changed — `git rev-list --count 8d2402c99..HEAD` is exactly 337 — so editing that file at all resets the height. Measured: `nbgv cloud -c -a` with the flag flipped emits `NBGV_Version=2.0.0.4992` instead of `2.0.337.4992`. Hence an MSBuild property in `version.props` rather than a `version.json` edit. ## No dump when the test host dies natively — addresses #4447 (step 1) On [run 34158052372](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/34158052372/job/101853936251) the Sessions test host was lost to a glibc `double free or corruption (out)` and produced no dump at all, leaving native memory corruption in the security stack with no call stack. Neither dump path the job already configures covers that class of failure: - `--blame-crash-dump-type mini` goes through the runtime's `createdump`, which the runtime invokes only for crashes it handles itself; a raw `SIGABRT` out of glibc's malloc consistency check is not one of them. - `--blame-hang-dump-type mini` asks the test host to dump itself over the diagnostics socket, so it needs a runtime healthy enough to service the request. On that run it answered `Collect dump was enabled but no dump file was generated`. A kernel core depends on neither, so this adds three steps to the Linux legs: 1. **Enable native core dumps** — point `core_pattern` at `/tmp/coredumps` (outside the workspace, so `Upload test results` cannot sweep a multi-gigabyte file in by accident) and relax `kernel.yama.ptrace_scope` so a follow-up can also attach `gdb -p` or `dotnet-dump collect` to a wedged host. 2. **Raise `RLIMIT_CORE`** on the shell that launches the test host. The limit is inherited rather than global, so it cannot be raised from the step above. 3. **Stage Linux core dumps** — on failure, extract a `gdb` backtrace as text and always keep it; keep the compressed core only while it stays under 2 GiB. All three warn rather than fail: losing the ability to collect a core is not a reason to stop the tests from running. Healthy runs write nothing. **This makes the next occurrence diagnosable; the double free itself is not fixed here.** That is step 2 of the issue and it is explicitly gated on having a dump — guessing at a certificate-lifetime change in the security stack would invert straight into a leak. See the comment on #4447 for a correction to that issue's ECC hypothesis, which the raw job log does not support. ## Verification - YAML parses; both bash blocks pass `bash -n`; the pwsh block parses. - Staging script exercised through every branch: no cores, two cores, oversize drop, and `sudo`/`gdb` unavailable — each exits 0. - `version.props` change verified to emit nothing to `$GITHUB_ENV` while leaving the computed version identical. - CRLF line endings preserved in both files. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
This commit is contained in:
romanett
@@ -127,6 +127,58 @@ runs:
|
||||
echo "OPCUA_PKCS11_PIN=1234"
|
||||
} >> "$GITHUB_ENV"
|
||||
|
||||
- name: Enable native core dumps
|
||||
# Issue #4447: on run 34158052372 the testhost was lost to a glibc abort -
|
||||
# `double free or corruption (out)` - and no dump was produced at all,
|
||||
# leaving a native memory-corruption bug in the security stack with no call
|
||||
# stack to work from. Neither dump path this action already configures can
|
||||
# be relied on for that class of failure:
|
||||
# --blame-crash-dump-type mini goes through the runtime's createdump,
|
||||
# which the runtime invokes only for crashes it handles itself; a raw
|
||||
# SIGABRT out of glibc's malloc consistency check is not one of them.
|
||||
# --blame-hang-dump-type mini asks the testhost to dump itself over the
|
||||
# diagnostics socket, so it needs a runtime healthy enough to service
|
||||
# the request. On that run it answered 'Collect dump was enabled but no
|
||||
# dump file was generated'.
|
||||
# A kernel core is written by the kernel and depends on neither, so it is
|
||||
# the one artifact that survives heap corruption. Nothing is written unless
|
||||
# a process actually dies on a signal, so healthy runs pay nothing.
|
||||
#
|
||||
# Cores land outside the workspace on purpose, so that 'Upload test
|
||||
# results' cannot sweep a multi-gigabyte file in by accident. 'Stage Linux
|
||||
# core dumps' below collects them deliberately, and only on failure.
|
||||
#
|
||||
# This step is diagnostics only, so it warns rather than fails: losing the
|
||||
# ability to collect a core is not a reason to stop the tests from running.
|
||||
if: ${{ runner.os == 'Linux' }}
|
||||
shell: bash
|
||||
run: |
|
||||
set -uo pipefail
|
||||
set +e # `shell: bash` runs with -e; each command is checked explicitly
|
||||
|
||||
if ! (sudo mkdir -p /tmp/coredumps && sudo chmod 1777 /tmp/coredumps); then
|
||||
echo '::warning::Could not create /tmp/coredumps; no core dumps will be collected.'
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# Ubuntu pipes cores to apport by default, which discards them for
|
||||
# processes it does not own. Write plain files instead.
|
||||
if ! sudo sysctl -w kernel.core_pattern='/tmp/coredumps/core.%e.%p'; then
|
||||
echo '::warning::Could not set kernel.core_pattern; a native abort will not leave a core dump.'
|
||||
fi
|
||||
|
||||
# yama defaults to 1 (a tracer may only attach to its own descendants).
|
||||
# createdump is forked by the target and traces its parent, which is not
|
||||
# that relation; the runtime asks for an exemption via PR_SET_PTRACER,
|
||||
# but relaxing the scope also lets `gdb -p` and `dotnet-dump collect`
|
||||
# attach to a wedged testhost, so a live follow-up is not blocked on it.
|
||||
if [ -e /proc/sys/kernel/yama/ptrace_scope ]; then
|
||||
sudo sysctl -w kernel.yama.ptrace_scope=0 \
|
||||
|| echo '::warning::Could not relax kernel.yama.ptrace_scope.'
|
||||
fi
|
||||
|
||||
echo "core_pattern=$(cat /proc/sys/kernel/core_pattern)"
|
||||
|
||||
- name: Build and test
|
||||
shell: pwsh
|
||||
# Inputs are passed through the environment rather than interpolated into
|
||||
@@ -146,6 +198,22 @@ runs:
|
||||
CI_PER_PROJECT_TIMEOUT: ${{ inputs.per-project-timeout }}
|
||||
CI_COVERAGE: ${{ inputs.coverage }}
|
||||
run: |
|
||||
# The core_pattern set by 'Enable native core dumps' only takes effect for
|
||||
# a process whose RLIMIT_CORE is non-zero, and the limit is inherited, not
|
||||
# global - so it has to be raised here, on the shell that launches the test
|
||||
# host, rather than in that step's own shell. Raising the hard limit needs
|
||||
# root; if that fails the run continues without a core, since the managed
|
||||
# blame dumps are unaffected either way.
|
||||
if ($IsLinux)
|
||||
{
|
||||
& sudo prlimit --pid $PID --core=unlimited
|
||||
if ($LASTEXITCODE -ne 0)
|
||||
{
|
||||
Write-Host "::warning::Could not raise RLIMIT_CORE (prlimit exited with code $LASTEXITCODE); a native abort will not leave a core dump."
|
||||
}
|
||||
Write-Host "RLIMIT_CORE (soft) for this shell: $(& bash -c 'ulimit -c')"
|
||||
}
|
||||
|
||||
./.github/scripts/run-dotnet-tests.ps1 `
|
||||
-Projects $env:CI_PROJECTS `
|
||||
-CustomTestTarget $env:CI_CUSTOM_TEST_TARGET `
|
||||
@@ -189,6 +257,78 @@ runs:
|
||||
}
|
||||
Write-Host "Staged $($files.Count) macOS crash report(s) to '$dst'."
|
||||
|
||||
- name: Stage Linux core dumps
|
||||
# Counterpart of 'Stage macOS crash reports' for the kernel cores enabled by
|
||||
# 'Enable native core dumps'. A core of a .NET test host runs to several
|
||||
# gigabytes, which is not worth uploading whole, and the part that actually
|
||||
# names the code that corrupted the heap is the native backtrace. So the
|
||||
# backtrace is extracted here and always kept; the core itself is kept only
|
||||
# when it compresses to something small enough to be worth transferring.
|
||||
# Both land in TestResults, which 'Upload test results' already publishes.
|
||||
if: ${{ failure() && runner.os == 'Linux' }}
|
||||
shell: bash
|
||||
run: |
|
||||
set -uo pipefail
|
||||
set +e # one unprocessable core must not abandon the rest
|
||||
|
||||
src=/tmp/coredumps
|
||||
dst=TestResults/linux-diagnostics
|
||||
# An artifact bigger than this costs more upload time than the evidence
|
||||
# is worth; the backtrace is kept regardless, so nothing is lost outright.
|
||||
max_compressed=$((2 * 1024 * 1024 * 1024))
|
||||
|
||||
shopt -s nullglob
|
||||
cores=("$src"/core.*)
|
||||
if [ ${#cores[@]} -eq 0 ]; then
|
||||
echo "No core dumps found in $src."
|
||||
exit 0
|
||||
fi
|
||||
|
||||
mkdir -p "$dst"
|
||||
if ! command -v gdb >/dev/null 2>&1; then
|
||||
echo 'gdb not present; installing it to extract backtraces.'
|
||||
sudo apt-get update -qq && sudo apt-get install -y --no-install-recommends gdb
|
||||
fi
|
||||
|
||||
for core in "${cores[@]}"; do
|
||||
name=$(basename "$core")
|
||||
archive=''
|
||||
echo "Found $core ($(stat -c %s "$core") bytes)"
|
||||
|
||||
if command -v gdb >/dev/null 2>&1; then
|
||||
# %e in core_pattern is the executable's base name, which for a
|
||||
# test host is always `dotnet`; pass the binary so gdb can resolve
|
||||
# symbols for the native frames.
|
||||
gdb -batch -nx \
|
||||
-ex 'set pagination off' \
|
||||
-ex 'info registers' \
|
||||
-ex 'thread apply all bt full' \
|
||||
"$(command -v dotnet)" "$core" \
|
||||
> "$dst/$name.backtrace.txt" 2>&1
|
||||
echo "Wrote $dst/$name.backtrace.txt"
|
||||
else
|
||||
echo "::warning::gdb unavailable; no backtrace extracted for $name."
|
||||
fi
|
||||
|
||||
if command -v zstd >/dev/null 2>&1; then
|
||||
zstd -q -3 -T0 -o "$dst/$name.zst" "$core" && archive="$dst/$name.zst"
|
||||
else
|
||||
gzip -1 -c "$core" > "$dst/$name.gz" && archive="$dst/$name.gz"
|
||||
fi
|
||||
|
||||
if [ -f "$archive" ]; then
|
||||
size=$(stat -c %s "$archive")
|
||||
if [ "$size" -gt "$max_compressed" ]; then
|
||||
echo "::warning::$(basename "$archive") is $size bytes; dropping it and keeping only the backtrace."
|
||||
rm -f "$archive"
|
||||
else
|
||||
echo "Staged $archive ($size bytes)"
|
||||
fi
|
||||
fi
|
||||
done
|
||||
|
||||
ls -la "$dst"
|
||||
|
||||
- name: Upload test results
|
||||
# always(): the results of a failed run are the reason to keep them.
|
||||
if: ${{ always() }}
|
||||
|
||||
@@ -26,6 +26,39 @@
|
||||
<FileVersion>$(NBGV_Version)</FileVersion>
|
||||
<PackageVersion>$(NBGV_NuGetPackageVersion)</PackageVersion>
|
||||
</PropertyGroup>
|
||||
<!--
|
||||
Do not let Nerdbank.GitVersioning publish cloud build variables from inside
|
||||
MSBuild. Its SetCloudBuildVersionVars target runs once per project, and on
|
||||
GitHub Actions the provider implements "set a variable" as an append to the
|
||||
file named by $GITHUB_ENV. Builds here are deliberately parallel across
|
||||
MSBuild nodes (see .github/scripts/run-dotnet-tests.ps1), so
|
||||
several nodes append GitAssemblyInformationalVersion / GitBuildVersion /
|
||||
GitBuildVersionSimple to that one file concurrently. Interleaved appends tear
|
||||
a line, and the runner then rejects the leftover fragment after the compile
|
||||
has already succeeded:
|
||||
|
||||
Build succeeded. 0 Error(s)
|
||||
##[error]Unable to process file command 'env' successfully.
|
||||
##[error]Invalid format '7'
|
||||
|
||||
('7' being the tail of a torn `GitBuildVersion=2.0.3x7.yyyyy`.) See issue
|
||||
#4458.
|
||||
|
||||
Nothing reads these three variables: every workflow and pipeline that
|
||||
consumes version information runs .azurepipelines/set-version.ps1 first, and
|
||||
`nbgv cloud -c -a` there writes the same Git* variables plus the NBGV_* set
|
||||
that version.props and common.props actually use - once, from a single
|
||||
process, with no concurrency. This leaves that script as the only writer.
|
||||
|
||||
Note this is deliberately an MSBuild property rather than
|
||||
cloudBuild.setVersionVariables in version.json: the version height is the
|
||||
number of commits since version.json last changed, so editing that file at
|
||||
all would reset the height to 0 and drop every produced version from
|
||||
2.0.<height> to 2.0.0.
|
||||
-->
|
||||
<PropertyGroup>
|
||||
<NBGV_SetCloudBuildVersionVars>false</NBGV_SetCloudBuildVersionVars>
|
||||
</PropertyGroup>
|
||||
<PropertyGroup>
|
||||
<GitVersionBaseDirectory>$(MSBuildThisFileDirectory)</GitVersionBaseDirectory>
|
||||
</PropertyGroup>
|
||||
|
||||
Reference in New Issue
Block a user