Skip to content

Conversation

@limbonaut
Copy link
Collaborator

@limbonaut limbonaut commented Jul 28, 2025

Remove dependency on libatomic.so.1 on Linux. This causes issues on some installations that don't have libatomic.so.1 installed (or GCC).

@limbonaut limbonaut added cherrypick:0.x Bug Something isn't working labels Jul 28, 2025
@limbonaut limbonaut marked this pull request as ready for review July 28, 2025 21:08
@limbonaut limbonaut requested review from jpnurmi and tustanivsky July 28, 2025 21:15
@jpnurmi
Copy link
Collaborator

jpnurmi commented Jul 29, 2025

Would we be able to satisfy it by linking to libatomic.a, or would the consuming app need to link?

Run pwsh ./scripts/update-doc-classes.ps1
ERROR: Can't open dynamic library: /home/runner/work/sentry-godot/sentry-godot/project/addons/sentry/bin/linux/x86_64/libsentry.linux.debug.x86_64.so. Error: /home/runner/work/sentry-godot/sentry-godot/project/addons/sentry/bin/linux/x86_64/libsentry.linux.debug.x86_64.so: undefined symbol: __atomic_store_16.
   at: open_dynamic_library (drivers/unix/os_unix.cpp:1060)
ERROR: Can't open GDExtension dynamic library: 'res://addons/sentry/sentry.gdextension'.
   at: open_library (core/extension/gdextension.cpp:722)
ERROR: Error loading extension: 'res://addons/sentry/sentry.gdextension'.
   at: load_extensions (core/extension/gdextension_manager.cpp:313)

@limbonaut
Copy link
Collaborator Author

As I understand it, you need to link libatomic when your operations cannot be implemented using lock-free instructions. I'm not sure why it still wants it.

Copy link
Collaborator

@jpnurmi jpnurmi left a comment

Choose a reason for hiding this comment

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

Ah right, so std::atomic<sentry_uuid_t> probably wasn't lock-free since it required libatomic? A manual mutex might not be as pretty but it gets the job done. 🙂 Alternatively, something like -Wl,-Bstatic -latomic -Wl,-Bdynamic or -l:libatomic.a might have also been a possibility to eliminate the runtime dependency.

@limbonaut
Copy link
Collaborator Author

limbonaut commented Jul 29, 2025

Seems the 16 bytes is the key here. Anything up to 8 should be lock-free on most architectures. We could include it as static, but as we don't test ABI compatibility yet, I chose safer option.

@limbonaut limbonaut merged commit 61ca2b2 into main Jul 29, 2025
45 checks passed
@limbonaut limbonaut deleted the fix/remove-explict-libatomic-dep-on-linux branch July 29, 2025 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something isn't working cherrypick:0.x

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants