You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
addons/openssl-binding/test very flaky on ARM #30786
(Edit: see comment below for recommendations if you are running into related problems on your ARMv7 platforms)
arm-fanned has been offline for quite some time now since they've needed some heavy work, nodejs/build#1840
but I'm bringing them back online now, and it seems that at least one thing has crept in while it was offline. addons/openssl-binding/test is flaky on the Pi 3's. So far I've only seen it fail there but it could be elsewhere too, there haven't been enough runs to be sure.
Of course it could be elsewhere, or in OpenSSL itself. I'll try and get you instructions for getting in and reproducing but it's a pretty complicated setup for both getting in and running stuff, so give me some time to sort that out. It will probably be made easier when you have nodejs/build onboarding and have your .ssh/config setup properly from Ansible so you have ip addresses, ssh keys and jump hosts all configured for you.
It seems like, in some way, the buffer->GetBackingStore() call in the addon is problematic; the addon crashes when the rest of the function is commented out, but doesn’t crash when that line is also commented out.
I’m still trying to find out more, but sadly my ability to read ARM assembly isn’t close to where it’s for x86/x64, so it might take a while (esp. given that re-compiling Node.js isn’t practical on the CI machines on which we run the tests).
We build the Node.js binary for arm-fanned CI with -march=armv7-a, but not the addons. Apparently, for std::shared_ptr these two options are ABI-incompatible (I’m assuming they use different kinds of atomics/locks depending on whether the flag is set, but I haven’t dug in that far). I don’t know what this looks like in the release CI.
I think this means we either have to find a way to hide this implementation detail – and that may be somewhat costly – or we have to pick one ABI, either -march=armv7-a or not, and then stick with that. Choosing to always enable the flag would imply dropping armv6 support entirely, instead of it being experimental, I think.
I suggested to @mhdawson that a workaround might be to tell addon authors to prefix their builds (likely using node-gyp) with CC="gcc -march=armv7-a" CXX="g++ -march=armv7-a". He thought this might make a good doc addition for users about this issue if we can show that's a workable solution.
I'll leave this in for now and maybe it'll demonstrate a temporary workaround that we can suggest to users. It's probably too late to include it in the 14.x release notes. It's also not ideal, node-gyp should be doing this itself as per @bnoordhuis' suggestion.
Perhaps it would be better to append to CFLAGS/CXXFLAGS instead, to avoid overriding the compiler: CFLAGS="$CFLAGS -march=armv7-a" CXXFLAGS="$CXXFLAGS -march=armv7-a"
So the current recommendations for people having trouble with std::shared_ptr on ARMv7 platforms:
Our official armv7l binaries available from nodejs.org are compiled explicitly for armv7-a
If you are using the Raspberry Pi toolchain to compile addons, (or possibly some other third-party toolchain) on your ARMv7 platform, then it's going to be compiling down to ARMv6. RPi does this to be maximally compatible with their hardware, including the recent Pi Zero which is still ARMv6.
There is some kind of ABI incompatibility when it comes to the use of std::shared_ptr between our armv7-a binaries and natively compiled armv6 (most likely armv6zk on RPi) binaries which can cause problems.
The workaround, for now at least, is to prefix any addon compiles with CFLAGS="$CFLAGS -march=armv7-a" CXXFLAGS="$CXXFLAGS -march=armv7-a".
This should will work by adding this in front of npm install or node-gyp, or whatever other process you're using to build addons.
This will make your addons build for ARMv7 only, they won't be compatible with ARMv6 platforms.
An alternative, if you want maximum compatibility and straightforward compiles is to use the unofficial armv6l binaries available at: https://unofficial-builds.nodejs.org/download/release/ - just be aware that these are not officially supported and you can't depend on them continuing to be available or not breaking. These binaries are compiled with armv6zk so should work across all Raspberry Pis with modern(ish) Linuxes.
Ping @rvagg ... does this need to remain open? I've been thinking that we need to have a better place of documenting these kinds of persistent issues. Maybe a doc/known-issues folder where they can be checked in...
Hm, maybe … my understanding was that that did not actually have the desired effect, but it might have solved this here as a side effect, which could be nice? I still wouldn’t be sure without actually verifying it :)
This issue has been marked as stale due to 210 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.
This issue has been automatically closed after 30 days of inactivity following its stale status (no activity for a total of 120 days).
If this is still relevant, feel free to reopen it or leave a comment with additional details so we can continue the discussion.
Activity
addaleax commentedon Dec 4, 2019
@rvagg Is there anything in particular that makes you suspect that commit?
addaleax commentedon Dec 4, 2019
valgrind doesn’t report anything suspicious for me.
Can you maybe point me to a host where this reproduces consistently on which I could debug?
rvagg commentedon Dec 6, 2019
@addaleax a very rough guess points me to that commit. arm-fanned has been offline since about July so I was looking at the history of openssl-binding during that time and that's it: https://github.com/nodejs/node/commits/master/test/addons/openssl-binding
Of course it could be elsewhere, or in OpenSSL itself. I'll try and get you instructions for getting in and reproducing but it's a pretty complicated setup for both getting in and running stuff, so give me some time to sort that out. It will probably be made easier when you have nodejs/build onboarding and have your .ssh/config setup properly from Ansible so you have ip addresses, ssh keys and jump hosts all configured for you.
richardlau commentedon Dec 7, 2019
Opened a PR to mark the test flaky for now: #30838
test: mark addons/openssl-bindings/test flaky on arm
test: mark addons/openssl-bindings/test flaky on arm
addaleax commentedon Dec 13, 2019
addaleax commentedon Dec 24, 2019
It seems like, in some way, the
buffer->GetBackingStore()call in the addon is problematic; the addon crashes when the rest of the function is commented out, but doesn’t crash when that line is also commented out.I’m still trying to find out more, but sadly my ability to read ARM assembly isn’t close to where it’s for x86/x64, so it might take a while (esp. given that re-compiling Node.js isn’t practical on the CI machines on which we run the tests).
addaleax commentedon Dec 24, 2019
We build the Node.js binary for arm-fanned CI with
-march=armv7-a, but not the addons. Apparently, forstd::shared_ptrthese two options are ABI-incompatible (I’m assumingthey use different kinds of atomics/locks depending on whether the flag is set,but I haven’t dug in that far). I don’t know what this looks like in the release CI.I think this means we either have to find a way to hide this implementation detail – and that may be somewhat costly – or we have to pick one ABI, either
-march=armv7-aor not, and then stick with that. Choosing to always enable the flag would imply dropping armv6 support entirely, instead of it being experimental, I think.@nodejs/build @nodejs/platform-arm
test: mark addons/openssl-bindings/test flaky on arm
30 remaining items
rvagg commentedon Apr 21, 2020
I suggested to @mhdawson that a workaround might be to tell addon authors to prefix their builds (likely using node-gyp) with
CC="gcc -march=armv7-a" CXX="g++ -march=armv7-a". He thought this might make a good doc addition for users about this issue if we can show that's a workable solution.I've added
V=1 CC='gcc -march=armv7-a' CXX='g++ -march=armv7-a'as a prefix to the entry into the docker container that builds addons on node-test-binary-arm-12+ and you can see, thanks toV=1, the execution in action: https://ci.nodejs.org/job/node-test-binary-arm-12+/5504/RUN_SUBSET=addons,label=pi3-docker/consoleFullI'll leave this in for now and maybe it'll demonstrate a temporary workaround that we can suggest to users. It's probably too late to include it in the 14.x release notes. It's also not ideal, node-gyp should be doing this itself as per @bnoordhuis' suggestion.
mscdex commentedon Apr 21, 2020
Perhaps it would be better to append to
CFLAGS/CXXFLAGSinstead, to avoid overriding the compiler:CFLAGS="$CFLAGS -march=armv7-a" CXXFLAGS="$CXXFLAGS -march=armv7-a"2020-04-21, Version 14.0.0 (Current)
rvagg commentedon Apr 22, 2020
Good call @mscdex, and that passes nicely through node-gyp. I've updated the CI job to use roughly this and it's working fine: https://ci.nodejs.org/job/node-test-binary-arm-12+/5525/RUN_SUBSET=addons,label=pi3-docker/consoleFull
rvagg commentedon Apr 22, 2020
So the current recommendations for people having trouble with
std::shared_ptron ARMv7 platforms:armv7-astd::shared_ptrbetween ourarmv7-abinaries and natively compiledarmv6(most likelyarmv6zkon RPi) binaries which can cause problems.CFLAGS="$CFLAGS -march=armv7-a" CXXFLAGS="$CXXFLAGS -march=armv7-a".npm installornode-gyp, or whatever other process you're using to build addons.armv6lbinaries available at: https://unofficial-builds.nodejs.org/download/release/ - just be aware that these are not officially supported and you can't depend on them continuing to be available or not breaking. These binaries are compiled witharmv6zkso should work across all Raspberry Pis with modern(ish) Linuxes.jasnell commentedon Jun 25, 2020
Ping @rvagg ... does this need to remain open? I've been thinking that we need to have a better place of documenting these kinds of persistent issues. Maybe a doc/known-issues folder where they can be checked in...
addaleax commentedon Jun 25, 2020
I would keep this open. This is not just a flaky test, that’s just a symptom of the underlying “real” problem.
Trott commentedon Feb 27, 2021
I haven't seen this test fail in a very long time. Is it at all likely that the cause of the failure has been fixed?
addaleax commentedon Feb 27, 2021
@Trott I don’t think anybody did anything to fix this, and I this isn’t the kind of issue that “magically” goes away…
richardlau commentedon Feb 27, 2021
Might #36634 have had an effect?
addaleax commentedon Feb 27, 2021
Hm, maybe … my understanding was that that did not actually have the desired effect, but it might have solved this here as a side effect, which could be nice? I still wouldn’t be sure without actually verifying it :)
github-actions commentedon Jun 27, 2026
This issue has been marked as stale due to 210 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.
github-actions commentedon Jul 28, 2026
This issue has been automatically closed after 30 days of inactivity following its stale status (no activity for a total of 120 days).
If this is still relevant, feel free to reopen it or leave a comment with additional details so we can continue the discussion.