12 ms·

I was burnt by the mutability of keys in go maps a few months ago, I'm not sure exactly how go handles it internally but it ended up with the map growing and du

I was burnt by the mutability of keys in go maps a few months ago, I'm not sure exactly how go handles it internally but it ended up with the map growing and duplicate keys in the key list when looking at it with a debugger. The footgun was that url.QueryUnescape returned a slice of the original string if nothing needed to be escaped so if the original string was modified, it would modify the key in the map if you put the returned slice directly into the map.

ncruces 9mo ago
Just how are you modifying strings? Cause that's your bug to fix.
- hiddendoom45 9mo ago
  That was probably done by fiber[1] the code specifically took the param from it in the function passed to the Get(path string, handlers ...Handler) Router function. c is the *fiber.Ctx passed by fiber to the handler. My code took the string from c.Param("name") passed it to url.QueryUnescape then another function which had a mutex around setting the key/value in the map. I got the hint it was slices and something modifying the keys when I found truncated keys in the key list. My guess is fiber used the same string for the param to avoid allocations. The fix for it is just to create a copy of the string with strings.Clone() to ensure it does not get mutated when it is used as a key. I understand it was an issue with my code, it just wasn't something I expected to be the case so it took several hours and using the debugger to find the root cause. Probably didn't help that a lot of the code was generated by Grok-4-Code/Sonic as a vibe coding test when I decided to go back a few months later and try and fix some of the issues I had myself. [1] https://github.com/gofiber/fiber https://github.com/gofiber/fiber
  - ncruces 9mo ago
    Go strings are supposed to be immutable. I see that fiber goes behind your back and produces potentially mutable strings behind your back: https://github.com/gofiber/utils/blob/c338034/convert.go#L18 https://github.com/gofiber/utils/blob/c338034/convert.go#L18 And… I actually don't have an issue with it to be honest. I've done the same myself. But this mutability should never escape. I'd never persist in using a library that would let it escape. But apparently… it's intentional: https://github.com/gofiber/fiber/issues/185 https://github.com/gofiber/fiber/issues/185 Oh well. You get what you ask for. Please don't complain about maps if you're using a broken library.
kbolino 9mo ago
This sounds like a bug, whether it be in your code, the map implementation, or even the debugger. Map keys are not mutable, and neither are strings.
- hiddendoom45 9mo ago
  This shouldn't be a race condition, reads were done by taking a RLock() from a mutex in a struct with the map, and defer RUnlock(), writes were similar where a Lock() was taken on the same mutex with a defer Unlock(). All these functions did was get/set values in the map and operated on a struct with just a mutex and the map. Unless I have a fundamental misunderstanding of how to use mutexes to avoid race conditions this shouldn't have been the case. This also feels a lot like a llm response with the Hypotheses section. edit: this part below was originally a part of the comment I'm replying to Hypotheses: you were modifying the map in another goroutine (do not share maps between goroutines unless they all treat it as read-only), the map implementation had some short-circuit logic for strings which was broken (file a bug report/it's probably already fixed), the debugger paused execution at an unsafe location (e.g. in the middle of non-user code), or the debugger incorrectly interpreted the contents of the map.
  - deleted 9mo ago
    [deleted]
  - deleted 9mo ago
    [deleted]
  - deleted 9mo ago
    [deleted]
arccy 9mo ago
That just means fiber is a bad library that abuses unsafe, resulting in real bugs.

ncruces 9mo ago

Just how are you modifying strings? Cause that's your bug to fix.

hiddendoom45 9mo ago
That was probably done by fiber[1] the code specifically took the param from it in the function passed to the Get(path string, handlers ...Handler) Router function. c is the *fiber.Ctx passed by fiber to the handler. My code took the string from c.Param("name") passed it to url.QueryUnescape then another function which had a mutex around setting the key/value in the map. I got the hint it was slices and something modifying the keys when I found truncated keys in the key list. My guess is fiber used the same string for the param to avoid allocations. The fix for it is just to create a copy of the string with strings.Clone() to ensure it does not get mutated when it is used as a key. I understand it was an issue with my code, it just wasn't something I expected to be the case so it took several hours and using the debugger to find the root cause. Probably didn't help that a lot of the code was generated by Grok-4-Code/Sonic as a vibe coding test when I decided to go back a few months later and try and fix some of the issues I had myself. [1] https://github.com/gofiber/fiber https://github.com/gofiber/fiber
- ncruces 9mo ago
  Go strings are supposed to be immutable. I see that fiber goes behind your back and produces potentially mutable strings behind your back: https://github.com/gofiber/utils/blob/c338034/convert.go#L18 https://github.com/gofiber/utils/blob/c338034/convert.go#L18 And… I actually don't have an issue with it to be honest. I've done the same myself. But this mutability should never escape. I'd never persist in using a library that would let it escape. But apparently… it's intentional: https://github.com/gofiber/fiber/issues/185 https://github.com/gofiber/fiber/issues/185 Oh well. You get what you ask for. Please don't complain about maps if you're using a broken library.

kbolino 9mo ago

This sounds like a bug, whether it be in your code, the map implementation, or even the debugger. Map keys are not mutable, and neither are strings.

hiddendoom45 9mo ago
This shouldn't be a race condition, reads were done by taking a RLock() from a mutex in a struct with the map, and defer RUnlock(), writes were similar where a Lock() was taken on the same mutex with a defer Unlock(). All these functions did was get/set values in the map and operated on a struct with just a mutex and the map. Unless I have a fundamental misunderstanding of how to use mutexes to avoid race conditions this shouldn't have been the case. This also feels a lot like a llm response with the Hypotheses section. edit: this part below was originally a part of the comment I'm replying to Hypotheses: you were modifying the map in another goroutine (do not share maps between goroutines unless they all treat it as read-only), the map implementation had some short-circuit logic for strings which was broken (file a bug report/it's probably already fixed), the debugger paused execution at an unsafe location (e.g. in the middle of non-user code), or the debugger incorrectly interpreted the contents of the map.
- deleted 9mo ago
  [deleted]
- deleted 9mo ago
  [deleted]
- deleted 9mo ago
  [deleted]

arccy 9mo ago

That just means fiber is a bad library that abuses unsafe, resulting in real bugs.