fix(nbdkit): disable malloc-debug checking - #18705
fix(nbdkit): disable malloc-debug checking#18705Tobias Brick (tobiasb-ms) wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The focused workaround is consistently rendered and validated by successful full test-suite builds.
Pull request overview
Disables legacy glibc allocator checking in nbdkit tests to prevent SymCrypt allocation/free mismatches while retaining memory perturbation.
Changes:
- Adds and applies the test-environment patch.
- Refreshes the rendered spec, release, and component lock.
- Preserves the complete test suite.
File summaries
| File | Description |
|---|---|
base/comps/nbdkit/0001-tests-disable-glibc-malloc-check.patch |
Defines the allocator-check workaround. |
base/comps/nbdkit/overlays/0003-disable-glibc-malloc-check.overlay.toml |
Registers the patch with rationale and tracking metadata. |
specs/n/nbdkit/0001-tests-disable-glibc-malloc-check.patch |
Contains the matching rendered patch. |
specs/n/nbdkit/nbdkit.spec |
Applies the patch and increments the release. |
locks/nbdkit.lock |
Refreshes the component input fingerprint. |
Review details
- Files reviewed: 4/5 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
/azp run |
|
Azure Pipelines: 2 pipeline(s) were filtered out due to trigger conditions. |
reuben olinsky (reubeno)
left a comment
reuben olinsky (reubeno)
left a comment
There was a problem hiding this comment.
The patch looks okay to me -- one quick blocking question regarding the referenced bug, and a non-blocking question.
| Date: Thu, 3 Sep 2026 20:17:00 +0000 | ||
| Subject: [PATCH] tests: disable glibc malloc check | ||
|
|
||
| The legacy glibc malloc checker interposes allocation APIs through |
There was a problem hiding this comment.
question(non-blocking): This characterizes the use of libc_malloc_debug as a "legacy" approach -- is that accurate? Is it really deprecated? (Is there a newer mechanism that's recommended?)
There was a problem hiding this comment.
You're right. I overgeneralized from glibc's removal of the deprecated malloc hooks and its description of libc_malloc_debug hook support as transitional. glibc.malloc.check itself is still supported, so I've removed “legacy” and now refer to glibc's malloc-debug checker.
| category = "azl-temp-workaround" | ||
| upstream-status = "unknown" | ||
| bugs = [ | ||
| { url = "https://dev.azure.com/mariner-org/mariner/_workitems/edit/23389" }, |
There was a problem hiding this comment.
question(blocking): Did you mean to leave in a reference to the internal bug here? If there's a GitHub-located bug filed against SymCrypt that might be the more appropriate reference.
There was a problem hiding this comment.
Yes, that's intentional. There are many other examples of doing this with an internal bug url. There is no SymCrypt issue filed.
There was a problem hiding this comment.
Okay, I see that, Tobias Brick (@tobiasb-ms), and won't block on this. But also I'm not convinced those are really the intended use of the bugs property here -- these aren't so much bugs as they are some internal tracking tickets for feature work.
If there are no upstream bugs and there's already sufficient explanation of the issue here, I don't know that the persistent reference is too useful. (And if there were additional useful context that's publishable in GitHub, we should see if we could do that instead.)
Tobias Brick (tobiasb-ms)
changed the title
8d62efc to
4dfee21
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The random perturbation value can be 256, causing some test runs to disable the only remaining malloc diagnostic.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 4/5 changed files
- Comments generated: 2
- Review effort level: Balanced
Remove glibc malloc-debug checking from the test environment because it splits SymCrypt allocations across libc and malloc-debug. Retain malloc perturbation and the full test suite. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
4dfee21 to
24ebd6c
Compare
|
/azp run |
|
Azure Pipelines: 2 pipeline(s) were filtered out due to trigger conditions. |
There was a problem hiding this comment.
🔵 Needs a closer look
The new overlay’s upstream disposition remains unassessed as unknown.
Review details
Suppressed comments (1)
base/comps/nbdkit/overlays/0003-disable-glibc-malloc-check.overlay.toml:7
- Please replace
unknownwith an assessed upstream disposition. This is the only overlay in the repository that leaves the status unknown, and the overlay metadata guidance requires new work to document how it can eventually be dropped. For this AZL-specific workaround, useinapplicableif it should simply disappear with the external SymCrypt fix, orneeds-upstream-hookif nbdkit should expose a supported way to disable this checker.
upstream-status = "unknown"
- Files reviewed: 4/5 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Removes glibc malloc-debug checking from the nbdkit test environment because it splits SymCrypt allocations across libc and malloc-debug. Retains malloc perturbation and the full test suite.
AB#23389
Validation: stage2 and stage2-prod builds pass all 442 tests (366 passed, 76 skipped).