Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add a stable ruby-asan variant? #682

Open
ivoanjo opened this issue Jan 2, 2025 · 5 comments
Open

Add a stable ruby-asan variant? #682

ivoanjo opened this issue Jan 2, 2025 · 5 comments

Comments

@ivoanjo
Copy link

ivoanjo commented Jan 2, 2025

Hey 👋

I've been meaning to ask this once Ruby 3.4 is out. Would it be possible to add a stable ruby-asan variant? (e.g. using stable 3.4 releases?).

My use-case is that we're using ruby-asan to test the datadog gem and we've been "bitten" a few times with our CI check starting to fail due to unrelated ruby-head changes.

This is annoying because it leaves us in a situation where this check is brittle, but we still don't want to disable it since it may catch issues.

Having a stable ruby-asan would be a really good middle ground, since we'd know that if it went from green to red, it was because of a change we made, not because ruby-head is, as it should be, sometimes bumpy ;)
@eregon
Copy link
Member

eregon commented Jan 2, 2025 •
edited
Loading

Do you know if there are any leaks with 3.4.1, like in #653 (comment) ?

I think the main difficulty here is the logic to build with ASAN is in https://github.com/ruby/ruby-dev-builder/blob/593dd0185840cd73696885fc1057090a691ae295/.github/workflows/build.yml#L137-L166 but releases are done with https://github.com/ruby/ruby-builder/ and ruby-build.
So not quite sure how to adapt that and I doubt I would have time for it.

Also I wouldn't want to create variants of every release, it's already a lot of builds per versions due to many platforms.

Maybe this could (should?) be done as a fork of ruby-builder or just a new repo creating that ASAN 3.4 build, and then support in setup-ruby to just pick a build from a URL. Then you could just use whatever you want to build it, it wouldn't need to be consistent and maintainers of setup-ruby wouldn't have to take the overhead of dealing with that.

ivoanjo added a commit to DataDog/dd-trace-rb that referenced this issue Jan 9, 2025
@ivoanjo
Add ostruct to Ruby 3.5 gemfile
de276c7 
This is needed to unblock the test-asan check. I'm working with
upstream ( ruby/setup-ruby#682 ) to see if
we can get an asan build based off a stable Ruby to avoid the
"flakiness" of testing with a Ruby based on ruby-head.
@ivoanjo
Copy link
Author

ivoanjo commented Jan 10, 2025 •
edited
Loading

Ooops I realized my e-mail filters were set up wrong and I missed your reply on this thread. Thanks for the patience 😅

Do you know if there are any leaks with 3.4.1, like in #653 (comment) ?

I don't think there are. We've been running with the current one based on Ruby-head for a few weeks now without issues related to memory leaks -- https://github.com/DataDog/dd-trace-rb/actions/workflows/test-memory-leaks.yaml .

We do ship a suppression file for warnings about threads -- https://github.com/DataDog/dd-trace-rb/blob/master/suppressions/lsan.supp but TBH I'm not 100% sure if that's a real one or just caused by forks (since cleaning up memory from old threads on fork is... weird probably).

I think the main difficulty here is the logic to build with ASAN is in (...) but releases are done with (...) ruby-build.
So not quite sure how to adapt that and I doubt I would have time for it.

Ah, you're right, I had completely missed that big difference. 🤔

Also I wouldn't want to create variants of every release, it's already a lot of builds per versions due to many platforms.

Yes +1 on this. I think having latest stable Ruby + latest ubuntu is more than enough -- I'm only looking to reduce the flakiness that dealing with ruby-head introduces for a CI step that ideally would break the build if it's failing.

Maybe this could (should?) be done as a fork of ruby-builder or just a new repo creating that ASAN 3.4 build, and then support in setup-ruby to just pick a build from a URL. Then you could just use whatever you want to build it, it wouldn't need to be consistent and maintainers of setup-ruby wouldn't have to take the overhead of dealing with that.

Yeap. Here's some thoughts on possible paths, based on your feedback:

  1. Create a new repo that builds the asan Ruby, have setup-ruby support pulling from it by url
  2. Work with upstream ruby-build to get support for setting the correct ASAN flags when building Ruby, allowing it to then easily be used in ruby-builder
  3. It occurred to me that ruby-dev-builder already has 99% of the machinery we need, it's just pointed at the wrong branch. Would you accept a patch that tweaks ruby-dev-builder to add a new asan-stable (asan-3.4? something else?) and adds a new matrix variant with the only difference being the Ruby tag/branch being cloned?

Any thoughts on which one is preferable? 3 seems the easiest to me, but maybe a bit ugly haha; I can look into 2 or 1 if you think those are better paths.

@eregon
Copy link
Member

eregon commented Jan 10, 2025

We do ship a suppression file for warnings about threads -- https://github.com/DataDog/dd-trace-rb/blob/master/suppressions/lsan.supp but TBH I'm not 100% sure if that's a real one or just caused by forks (since cleaning up memory from old threads on fork is... weird probably).

Right, that's what I was thinking, I think it would be good if no suppressions would be needed, so it's easier to setup.
But that would kind of need some test in CRuby's CI to avoid regressions there.
And so from my POV using the ASAN builds from memory leaks doesn't seem ready/supported upstream until that happens.

OTOH it's just one suppression you have and having it based on a known release would avoid changes.

  1. could be nice, then I only have to review adding support for a url in setup-ruby (or implement it myself but I don't know if I'll have time) and it's something generally useful not just for this case
  2. I feel that's out of ruby-build's concern/job. But of course one can use ruby-build with custom configuration flags and install extra packages etc
  3. Yes, I think that's worth exploring. However there is no point to rebuild 3.4.1 with ASAN on every ruby/ruby commit/every day.
    So probably it could be done just once or so, and saved to a separate place/release.
    Also what should happen when 3.4.2 comes out? It needs to be low effort.

I agree 3 seems easiest. Can you look into it and try to figure out the details?

@ivoanjo
Copy link
Author

ivoanjo commented Jan 10, 2025

Yeah there's a bit of "building on top of not entirely-solid-foundations" for the overall state of ASAN in Ruby/the ecosystem/toolchains, etc. Yet we've been able to spot a few issues before they blow up in prodction with this tooling in dd-trace-rb so I think overall from our side we have enough value from it to be worth keeping in the current state.

I agree 3 seems easiest. Can you look into it and try to figure out the details?

Yes! I should be able to take a stab at it next week and report back

ivoanjo added a commit to DataDog/ruby-dev-builder that referenced this issue Jan 17, 2025
@ivoanjo
Add 3.4-asan builds
14229b5 
**What does this PR do?**

This PR introduces a new "3.4-asan" build, based on the existing
asan builds, but just pointed at whatever's the latest tag in the
3.4 series.

**Motivation:**

The intention of "3.4-stable" is to provide the latest up-to-date
stable Ruby, so that we can reliably use it as a breaking CI step.

As discussed in ruby/setup-ruby#682, the
current ruby-asan builds are a bit of a "sharp edge" when used in CI
because they may break due to changes that are completely unrelated
to asan.

Building asan rubies is a bit awkward still, as e.g. ruby-build and
other version managers don't have support for it, and it requires
very modern versions of specific system tools (e.g. clang).

**Additional Notes:**

After some back-and-forth, the changes are reasonably minimal.

In particular, I decided to not touch the logic that determines
weather there's a more recent commit to build or not. This does
mean that if ruby master sees no commits, but there's a new 3.4
stable release, this won't be picked up immediately; and it
also means that if there's a new master commit and no change
to the 3.4 branch we still rebuild 3.4-asan.

My thinking is that the extra complexity to individually take
care of the caching for both branches is not worth the trouble vs
doing some extra rebuilds for 3.4-asan.

Let me know if you're not convinced, and I can change that.

**How to test the change?**

I've built this in the downstream fork, and manually downloaded
the resulting Ruby and it seems to be in good shape and with
asan working fine.

* Successful run:
  https://github.com/DataDog/ruby-dev-builder/actions/runs/12827351740
* Resulting builds:
  https://github.com/DataDog/ruby-dev-builder/releases/tag/v20250117.103455
@ivoanjo ivoanjo mentioned this issue Jan 17, 2025
Open
@ivoanjo
Copy link
Author

ivoanjo commented Jan 17, 2025

Here's my attempt at 3: ruby/ruby-dev-builder#13 . LMK your thoughts :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@eregon @ivoanjo