Locking triomphe is the wrong approach to preserving MSRV

[aumetra]

The recent release locked the triomphe crate version to >=0.1.3, <0.1.12 which kinda works(?) but isn't a good solution.
IMO it's fine if, by default, the MSRV breaks if there is a simple way of getting the MSRV to compile (in this case, by adding =0.1.11 to the manifest of projects that absolutely need the old Rust version).

There have been prior examples where a version lock like this has broken compilation for many downstream users, such as with the *-dalek crates, where zeroize used to be locked (1, 2, 3, 4).

MSRV should, in my opinion at least, not block new versions if there is a simple fix to getting that Rust version running.

[tatsuya6502]

Thank you for your comment. We know it is not an ideal approach to lock dependency in older versions , but we had no other options.

• triomphe@v0.1.12 requires Rust 1.76.0 (2024-02-08) or newer, which conflicts with our MSRV policy:

  • It will keep a rolling MSRV policy of at least 6 months.

  • As of today, our policy allows Rust 1.75.0 (2023-12-28).
• It seems triomphe@v0.1.12 conflicted at least two downstream moka user's MSRV. (Pin triomphe to keep the MSRV #426)
  • Raising moka's MSRV does not solve their problems.

Locking dependency in older versions may become a problem in the future, but no downstream user has reported yet. Meanwhile, some downstream users already reported problems with a newer version of the same dependency.

Anyway. To prevent the potential future problem, I will remove triomphe from moka's dependency. moka uses a small portion of triomphe code base, which seems very stable. And triomphe is a fork of std::sync::Arc. I will copy necessary part of triomphe or std::sync::Arc code into moka, and modify it to meet our needs. (Their OSS licenses allow it)

[aumetra]

Is there any reason why those downstream users can't pin triomphe themselves?
I don't see why they would need moka to pin versions for them (and potentially breaking downstream uses of moka because of this requirement) if they can just fix it themselves.

It also seems like this approach of making users pin crate versions themselves isn't foreign to moka crates, looking at the .ci_extras directory over on the mini-moka repo (https://github.com/moka-rs/mini-moka/blob/main/.ci_extras/pin-crate-vers-msrv.sh)

[tatsuya6502]

Is there any reason why those downstream users can't pin triomphe themselves?

No. Anybody can pin trionmphe version in their Cargo.lock by using cargo update -p PACKAGE --precise VERSION. However, Cargo ignores Cargo.lock provided by dependencies, so if they distribute library crates, pinning in Cargo.lock has no effect for their downstream users.

I don't see why they would need moka to pin versions for them

The main purpose to pin triomphe in moka is to keep our MSRV, not to keep their MSRVs. We needed it for ourselves in the first place.

Our policy ensures moka to compile with Rust versions at least 6 months old. triomphe@v0.1.12's MSRV is Rust 1.76.0 (2024-02-08), and at the time v0.1.12 was released, Rust 1.76.0 was only 3 moths old. If moka depended on triomphe@v0.1.12, it meant its MSRV was broken.

As for dashmap in mini-moka, I did not pin it in Cargo.toml but pinned it in Cargo.lock only for our CI. This is because the MSRV of the latest dashmap (5.x) did not conflict with mini-moka's 6-month MSRV policy. dashmap@v5.5.3's MSRV is Rust 1.65.0 2022-11-03. So I was thinking if mini-moka user reports mini-moka's MSRV Rust 1.61.0 2022-05-19 is broken, I would simply raise the MSRV to Rust 1.65.0.

[tatsuya6502]

As I said before, I agree with you that locking triomphe in Cargo.toml is a wrong approach.

We know it is not an ideal approach to lock dependency in older versions , but we had no other options.

Now it is August 2024 and Rust 1.76.0 is 6 months old. We now have following options:

1. Raise moka's MSRV from 1.65.0 to 1.76.0 and unpin triomphe version in Cargo.toml.
2. Remove triomphe from moka's dependency. Take necessary codes from triomphe into moka.

I will take 2. because moka uses only a small portion of triomphe code base, which seems very stable.

After writing my first comment on July 13th, 2024, I did some experiments to take necessary triomphe code into moka. I was able to solve compile errors, but I was thinking to clean the code up further. Then I had no time to make any progress. I am hoping I will have some time in mid September to finish it.

[aumetra]

Ah, don't worry. I didn't mean to annoy you, but if you need help with integrating the vendored code, I'd be happy to help out.

[GnomedDev]

I don't think vendoring is the correct decision in any situation. How about talking to upstream about lowering their MSRV and committing your cleanups upstream?

[aumetra]

Seems like their MSRV bump was to make the behaviour more consistent/correct. I'm pretty sure they wouldn't reintroduce those kinds of things (which is a, quote, "longrunning potential soundness issue that got fixed with a new API") "just" to support an around half a year old compiler version.

Issue commenting about this: Manishearth/triomphe#93 (comment)
MSRV bump PR: Manishearth/triomphe#93

[GnomedDev]

Okay, so even if this library vendored triomphe with a lower MSRV there would be soundness problems. I personally don't see any issue with the current situation of MSRV 1.76 and using upstream triomphe.

[tatsuya6502]

Thanks for sharing your concerns.

When we locked the triomphe version to an older one, I reviewed relevant codes and concluded that both moka and mini-moka were not affected by the soundness problem. I believe it would occur only when storing a zero sized type in triomphe::Arc and we do not store such a type.

triomphe started as a fork of std::sync::Arc to remove the weak references from Arc. We only want that version of Arc without weak references to reduce memory overhead. We use a few number of Arcs per cached entry and the type of the weak reference counter is usize (8 bytes in 64 bit systems).

Now triomphe has many added features that we do not need, so I thought this would be a good time to move from triomphe to a simple fork of std::sync::Arc.

I do not have time right now, but I am still hoping I will have some time to work on it in coming weekend. So please let me try it.

[GnomedDev]

Rust will dead code eliminate any features not used, and forking will just mean you no longer get any fixes and maintenance... alongside leading to duplicate code being compiled if someone does bring triomphe in to their dependency tree somehow. This just seems like a very bad idea all around to me.

[vinayakb]

Thanks for the detailed analysis of the issue. Currently, I am facing a problem with being able to use moka in the same workspace with swc_* libraries due to the triomphe versioning issue. Any update on #456 that could help here? Thanks.

[tatsuya6502]

Hi. Sorry for the big delay on this issue. #456 is actually ready to merge, but I wanted to run some performance tests to ensure it will not introduce any problems. I will do it this week, and I hope I can release the next version of moka very soon.

---

I had been having health problems since last summer and it made me unable to participate in OSS activities. I had to concentrate on my full time job.

In early December, I took a series of tests and the cause of the problem was found. It turned out to be a side effect of a medication I had been taking for a year. I was in the early stage of rhabdomyolysis. My doctor switched me to a different medication and I am feeling much better now.

[tatsuya6502]

OK. I ran the performance tests on #456, and am more confident on my implementation.

Just FYI, in #456, I implemented our Arc type called MiniArc from scratch instead of vendoring the triomphe source code and modifying it. This made MiniArc implementation clean and very small (~100 LoC including lines only having }).

In last September, I measured compilation time on a real world project, and concluded that the compilation time increase by MiniArc should be negligible.

Also, in MiniArc, AtomicU32 is used for the reference counter, instead of AtomicUsize. moka heavily uses Arc so it can reduce runtime memory overhead on 64-bit platforms.

• moka's MiniArc has 1 × AtomicU32 (4 bytes + size of the value).
• triomphe:Arc has 1 × AtomicUsize (8 bytes on 64-bit platform + size of the value).
• std::sync::Arc has 2 × AtomicUsize (16 bytes on 64-bit platform + size of the value).

[tatsuya6502]

Published moka@v0.12.9 to crates.io.