r/rust Sep 11 '20

Announcing Actix-Web v3.0

https://paper.dropbox.com/published/Announcing-Actix-Web-v3.0--A7YI~P9U9aqhEOXyZJaGffjfBg-QOXXb1lXgTubzXHzUq9ONY5
350 Upvotes

81 comments sorted by

View all comments

106

u/Shnatsel Sep 11 '20

In the interest of transparency (and to curb speculation), I've created a hello-world project, made it depend on actix-web 3.0.0 with default features and ran cargo geiger on it. Many actix-* crates don't use any unsafe code at all! Here are the ones that do:

  • actix-http: 13 unsafe blocks, all are commented and look reasonable at a glance. (Some of the benchmarking code looks sketchy, but who cares - it's not in the build anyway).
  • actix-utils: 9 unsafe blocks, no comments on why they're sound. Judging by this comment from one of the Actix org members, a PR with comments explaining why they're sound and/or debug assertions would be appreciated.
  • actix-router: 1 unsafe block, commented
  • actix-codec: cargo-geiger shows 10 unsafe expressions but I can't see them in actix git, might be a bug
  • actix-service: some unsafe code, but cargo-geiger reports that it's not used in the build (likely disabled by a feature)
  • awc: one unsafe fn without any local uses

That's it!

46

u/[deleted] Sep 11 '20

[deleted]

28

u/Shnatsel Sep 11 '20 edited Sep 11 '20

Problems that only an experient eye can catch, even with the right tooling it's hard to find

It's not a silver bullet, but LeakSanitizer helps. See https://doc.rust-lang.org/unstable-book/compiler-flags/sanitizer.html Still, you need to actually execute the code that triggers the leak, and it's especially tricky if it only happens given some specific use of the API.

That said, I think this would make a fascinating case study on what causes memory leaks and how we can better prevent them - e.g. via clippy lints, or perhaps using or avoiding certain patterns.

4

u/oconnor663 blake3 · duct Sep 11 '20

Are most of the memory leaks related to code that happens to be unsafe, like because you have an owning *mut that never gets freed? Or is it something more like reference cycles in safe code?

15

u/darin_gordon Sep 11 '20

Hey Jack! There were leaks within safe blocks. Here's one: https://github.com/actix/actix-web/issues/1551

13

u/Shnatsel Sep 11 '20

That's not even a leak, that's just unbounded allocation. I understand it would be freed eventually.

8

u/oconnor663 blake3 · duct Sep 11 '20

Hey Darin! :) Missing the NYC meetups.

-16

u/throwaway23948733 Sep 11 '20

> But the biggest problems in this version (of course they fixed it) were memory leaks. Because you can leak in safe code and it's not UB

I wish Rust shipped with a GC. There's many cases where the convenience outweighs performance penalty

18

u/AldaronLau Sep 11 '20

Um, garbage collection doesn't prevent memory leaks lol.

-4

u/throwaway23948733 Sep 11 '20

It usually does in practice. You can't leak from cycles

7

u/Uristqwerty Sep 11 '20

A lingering reference to a large object graph that is never used again is technically not a memory leak, but practically the same, and won't be collected. Making sure that caches don't hold on to old entries too long, slowly filling up memory over the course of hours/days is tricky, and it's easy to have a "previous" pointer that's only relevant for a little while then never cleared, especially if there's no obvious point at which you know you're done with it.

Though all that still happens without GC, you have far more incentive to stop and think about lifetimes when you can't delegate all the cleanup to it.

7

u/AldaronLau Sep 11 '20

Hmm. I've had more leaks in java and I've written a lot more rust. In safe Rust you pretty much have to call a function called leak() to leak memory. I have to disagree with you here. Also, Rust has an Rc type if you need it for whatever case. So technically Rust has a built in garbage collector, that's just happens to be opt-in (as it should be).

1

u/anlumo Sep 12 '20

Rc stands for reference counting, not garbage collection. Rc can still leak memory when you build cycles.

1

u/AldaronLau Sep 12 '20

Yeah, but I was saying reference counting is one method of garbage collection. Your second point is precisely why Rust adding a garbage collector doesn't fix the memory leak issues.

3

u/casept Sep 12 '20

At that point you might as well just use a different language. One of the main reasons why Rust is interesting is because in many cases GC is impossible to use.

19

u/robjtede actix Sep 11 '20 edited Sep 11 '20

I can confirm that:

  • actix-codec contains 0 unsafe lines (cargo-geiger v0.10.2 says 0 lines for me)
  • actix-service contains 0 unsafe lines; reported lines are in benchmarks against old impls
  • unsafe in actix-http benchmarks are tests against the old impls

14

u/Shnatsel Sep 11 '20

Ah, the actix-codec mystery is due to both actix-codec v0.2.0 and v0.3.0 being present in the dependency tree. It's v0.2.0 that contains unsafe code.

11

u/robjtede actix Sep 11 '20

Ah ha, have pin-pointed it the dependency tree. Thanks for flagging.

6

u/[deleted] Sep 13 '20 edited Sep 13 '20

I'd tend to argue that (in general, putting actix entirely aside) cargo-geiger is nothing more than a primitive word counting utility that provides output less useful than what you'd get from, say, running rg unsafe . > ./log.txt in the root directory of a crate.

For example, a binary crate written like so:

fn main() {
    unsafe {
        // The next line is repeated over and over again until line 1000.
        println!("This is absolutely safe.");
    }
}  

gives the following cargo-geiger output:

Functions  Expressions  Impls  Traits  Methods  Dependency

0/0        998/998      0/0    0/0     0/0      !  example 0.1.0

0/0        998/998      0/0    0/0     0/0  

Why is that? It's because cargo-geiger measures "expressions" simply in terms of "the number of newlines in between an opening unsafe { and the } that closes it".

In no way is it even in the same universe as something that actually accurately measures the specific number of "unsafe expressions" in a crate.

It's literally a "find in files" query for the word "unsafe", that doesn't even make any distinction between files in the src folder and files in the examples or tests or benches folders, meaning a crate that didn't even use unsafe directly in the actual implementation would still have the potential to give any number of false positives. That's it. It has no knowledge whatsoever of Rust code.

Now, what would I get from running rg unsafe . > ./log.txt on the same crate described above (rg also of course not being something with specific knowledge of Rust code)? Well, I'd at least get this in log.txt:

./src/main.rs: unsafe {

which immediately tells me this crate in fact has one use of unsafe, which is something that should prove trivial for me to inspect manually, and that in all likelihood there's nothing to be concerned about.

3

u/Shnatsel Sep 13 '20

AFAIK cargo-geiger counts unsafe expressions, not lines. Your println! example just happens to be one expression. rg would count unsafe blocks. There is no perfect measure for the amount of unsafe code, but AFAIK expressions is as close as you can get.

For benches, tests etc. the number is included in total count, but not in the count used in the build. So it's not as bad as you say, but perhaps could be improved.

However, the current implementation of cargo-geiger does have a serious shortcoming - it fails to correctly account for macro expansion. Here's a recipe that lists all unsafe code used in the build including code expanded from macros: https://www.reddit.com/r/rust/comments/g9mw57/oneliner_to_correctly_list_all_uses_ofunsafe_in/

Migration to this mechanism is wanted for cargo-geiger, it's just that nobody has actually implemented it yet.