KEMBAR78
Add Liberica to the CI on Alpaquita musl by krk · Pull Request #1466 · async-profiler/async-profiler · GitHub
Skip to content

Conversation

krk
Copy link
Contributor

@krk krk commented Sep 8, 2025

Motivation and context

We can consider including more distributions in the GHA CI.

How has this been tested?

CI


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@apangin
Copy link
Member

apangin commented Sep 8, 2025

What value does this particular configuration add? Do you know any errors that other configurations don't catch?

@krk
Copy link
Contributor Author

krk commented Sep 9, 2025

Testing a commit that could potentially fail.

@krk krk changed the title Add Liberica to the CI on AL2023 Add Liberica to the CI on Alpaquita musl Sep 9, 2025
@krk
Copy link
Contributor Author

krk commented Sep 9, 2025

Please see https://github.com/async-profiler/async-profiler/actions/runs/17581662920/job/49939353504?pr=1466 for failures about malloc tracer with the gcc version in alpaquita.

@krk
Copy link
Contributor Author

krk commented Sep 9, 2025

All 4 failures are about calloc in nativemem tests.

NativememTests.canAgentTraceMallocCalloc (args: once) (agentArgs: start,nativemem,total,collapsed,file=%f)
NativememTests.canAgentTraceMallocCalloc/VM (args: once) (agentArgs: start,nativemem,total,cstack=vm,collapsed,file=%f)
NativememTests.canAsprofTraceAllNoLeak
NativememTests.canAsprofTraceMallocCalloc

Looks like the same root cause as #1226 (comment), cannot sample calloc.

Note that the calloc_hook_dummy workaround not applicable in this case.

@Baraa-Hasheesh
Copy link
Contributor

@krk I've checked the issue here & the root cause for the failures is that on Alpaquita musl calloc doesn't call malloc internally

        if (OS::isMusl()) {
            // On musl, calloc() calls malloc() internally, and posix_memalign() calls aligned_alloc().
            // Use dummy hooks to prevent double-accounting. Dummy frames from AP are introduced
            // to preserve the frame link to the original caller (see #1226).
            cc->patchImport(im_calloc, (void*)calloc_hook_dummy);
            cc->patchImport(im_posix_memalign, (void*)posix_memalign_hook_dummy);
        } else {
            cc->patchImport(im_calloc, (void*)calloc_hook);
            cc->patchImport(im_posix_memalign, (void*)posix_memalign_hook);
        }

We assume that if MUSL that calloc would always call malloc internally which seems to be incorrect in this case

Just change the cc->patchImport(im_calloc, (void*)calloc_hook_dummy); to cc->patchImport(im_calloc, (void*)calloc_hook); & it should work (Not a proper fix, but you can try it)

@apangin
Copy link
Member

apangin commented Sep 10, 2025

It turns out that in this Alpaquita docker image musl is statically linked with mimalloc (!) which behaves differently from musl's built-in malloc implementation. So, it's not enough to check if the current process runs with glibc or musl, because libc itself can be built with an alternative implementation :-/

image: public.ecr.aws/bellsoft/alpaquita-linux-gcc:15.2-musl
asprof-binaries-job: linux-x64
before-checkout: |
apk add liberica21-jdk util-linux-misc
Copy link
Member

Choose a reason for hiding this comment

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

Instead of installing these every time, wouldn't it be better to prepare an image with all the tools in place? That's what we do for other OS distributions.
This is especially relevant in the light of HTTP 504 failures during apk add I saw in the previous runs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do after #1502 is merged.

@krk
Copy link
Contributor Author

krk commented Sep 30, 2025

Force-pushed to resolve the conflict.

@apangin apangin merged commit 861f4f4 into async-profiler:master Sep 30, 2025
24 of 25 checks passed
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.

3 participants