Live data from Hacker News

Finding and fixing Ghostty's largest memory leak

mitchellh.com

71–80 of 152 posts

Re: Finding and fixing Ghostty's largest memory leak

#72

This is great news! Well done to everyone who helped sort it out. It was a problem noted by users in a thread here just last week, https://news.ycombinator.com/item?id=46460319 While Claude Code might have been the reason this bug became triggered by more people, there are some of us who were hitting it without ever having used Claude Code at all. Maybe the assumption about what makes a page non-standard, isn't as bl…

> Probably not a huge deal, but it does seem the fix will needlessly delete and recreate non-standard pages in the case where the new page needs to be non-standard, and the oldest one (that needs to be pruned) already is non-standard and could be reused. This is addressed in the blog post. It is how the PageList has always worked, and also how it worked before with the bug, because during capacity adjustment we would…

How come this isn't released as a hotfix / out of band patch but will follow the standard release cycle in March?

Re: Finding and fixing Ghostty's largest memory leak

#73
post #61

Earlier quoted context omitted.

Thank you. I think each of my options are pretty trivial in C. I guess what I'm not understanding for #3 is if size is immutable, how the size changed which caused the issue? The post said they changed the size of the page without changing the underlying size of the allocated memory. To me this is the big issue. There was a desync in information where the underlying assumption is that size tells you where the data ca…

I'm not 100%, but my understanding was that the non standard pages are always larger than the standard pages. If you need more than a standard page, you always get a freshly allocated non standard page. But when one was released, it was being treated as though it was standard sized. The pool would then reuse that memory, but only at a standard size. So every released non standard page leaked the difference between wh…

Yes that last point was what I meant. I see no reason that the metadata's size field should get updated without some realloc of the memory it points to. I think I'll need to look into the actual code to see what's going on there, though, because we may both be misunderstanding. It just seems very error prone to categorize how you free memory based on a field that by the time you get to `free` has no guaranteed relationship with where the memory came from. I think that should be fixed. What was done in the blog is more of a band-aid imo.

Re: Finding and fixing Ghostty's largest memory leak

#75

This is great news! Well done to everyone who helped sort it out. It was a problem noted by users in a thread here just last week, https://news.ycombinator.com/item?id=46460319 While Claude Code might have been the reason this bug became triggered by more people, there are some of us who were hitting it without ever having used Claude Code at all. Maybe the assumption about what makes a page non-standard, isn't as bl…

As a side note - Claude Code is making the CLI attractive in a renewed fashion - more than anything else did it last 20years.

Re: Finding and fixing Ghostty's largest memory leak

#77

I've been following the development of Ghostty for a while and while I have the feeling that there is a bit of over-engineering in this project, I find this kind of bug post mortem to be extremely valuable for anyone in love with the craft.

Over-engineered in what way?

It’s just a feeling, man.

Re: Finding and fixing Ghostty's largest memory leak

#78
post #67

Earlier quoted context omitted.

It's already released in the latest nightly build.

Are the nightly releases the expected way to get timely bugfixes?

That is how software releases generally work. AFAICT this is not a bug with broad impact or security implications.

Re: Finding and fixing Ghostty's largest memory leak

#79
This feels like a case of guessing at something you could know. There are two types of allocations that each have a size and free method. The free method is polymorphic over the allocations type. Instead of using a tag to know absolutely which type an object it is you guess based on some other factor, in this case a size invariant which was violated. It also doesn't seem like this invariant was ever codified otherwise the first time a large alloc was modified to a standard size it would've blown up. It's worth asking yourself if your distinguishing factor is the best you can use or perhaps there is a better test. Maybe in this case a tag would've been too expensive.

Re: Finding and fixing Ghostty's largest memory leak

#80

Earlier quoted context omitted.

The specific language feature you want if you insist that you don't want this kind of leak is Linear Types. Rust has Affine Types. This means Rust cares that for any value V of type T, Rust can see that we did not destroy V twice (or more often). With Linear Types the compiler checks that you destroyed V exactly once, not less and not more. However, one reason I don't end up caring about Leak Safety of this sort is t…

The issue isn’t linked list vs dequeue but type confusion about what was in the container. They didn’t forget to drop it - they got confused about which type was in the list when popping and returned it to the pool instead of munmap. The way to solve this in Rust would be to put this logic in the drop and hide each page type in an enum. That way you can’t ever confuse the types or what happens when you drop.

Was going to say this, but I don't think anyone actually wants to hear that Rust actually would have helped here.

As you're saying, the bug was the equivalent of an incorrectly written Drop implementation.

Nothing against Zig, and people not using Rust is just fine, but this is what happens when you want C-like feel for your language. You miss out on useful abstractions along with the superfluous ones.

"We don't need destructors, defer/errdefer is enough" is Zig's stance, and it was mostly OK.

Impossible to predict this kind of issue when choosing a project language (and it's already been discussed why Zig was chosen over Rust for Ghostty, which is fine!), so it's not a reason to always choose Rust over Zig, but sometimes that slightly annoying ceremony is useful!

Maybe some day I'll be smart enough to write Zig as a default over Rust, but until that day I'm going to pay the complexity price to get more safety and keep more safety mechanisms on the shotgun aimed at my foot. I've got plenty of other bugs I can spend time writing.

Another good example is the type vs type alias vs wrapper type debate. It's probably not reasonable to use a wrapper type every single time (e.g. num_seconds probably can probably be a u32 and not a Seconds type), but it's really a Rorschach test because some people lean towards one end versus the other for whatever reason, and the plusses/minuses are different depending on where you land on the spectrum.

[EDIT] also some good discussion here

https://ziggit.dev/t/zig-what-i-think-after-months-of-using-...

Post reply on HN