Remix.run Logo
hu3 5 days ago

That code would never pass a human pull request review. It doesn't even pass AI code review with a simple "review this code" prompt: https://chatgpt.com/share/68829f14-c004-8001-ac20-4dc1796c76...

"2. Shared buffer causes race/data reuse You're writing to buf, getting buf.Bytes(), and sending it to the channel. But buf.Bytes() returns a slice backed by the same memory, which you then Reset(). This causes line in processData to read the reset or reused buffer."

I mean, you're basically passing a pointer to another thread to processData() and then promptly trying to do stuff with the same pointer.

nemothekid 5 days ago | parent | next [-]

If you are familiar with the internals of bytes/buffer you would catch this. But it would be great for the compiler to catch this instead of a human reviewer. In Rust, this code wouldn't even compile. And I'd argue even in C++, this mistake would be clearer to see in just the code.

TheDong 5 days ago | parent | prev [-]

> I mean, you're basically passing a pointer to another thread to processData()

And yet, "bytes.Buffer.ReadBytes(delim)" returns a copy of the underlying data which would be safe in this context.

The type system does not make it obvious when this is safe or not, and passing pointers you own across channels is fine and common.

> That code would never pass a human pull request review

Yes, that was a simplified example that a human or AI could spot.

When you actually see this in the wild, it's not a minimal example, it's a small bug in hundreds of lines of code.

I've seen this often enough that it obviously does actually happen, and does pass human code review.