Worked review 4: a generic cache with TTL

Cache[K, V] คือ generic cache เล็กๆ ที่มีเวลาหมดอายุ — Set เก็บค่าพร้อม expiry, Get ควร return false เมื่อ expiry นั้นผ่านไปแล้ว มัน compile ผ่าน, go vet ไม่บ่นอะไรเลย, golangci-lint run ก็ไม่บ่นอะไรเลย, และมันรันได้ มันมี bug จริง 4 ตัว ตัวหนึ่งเป็น fatal error จริงภายใต้การใช้งานแบบ concurrent จริง

The program

Open in Playground

package main

import (
  "fmt"
  "time"
)

type entry[V any] struct {
  value V
  exp   time.Time
}

type Cache[K comparable, V any] struct {
  data map[K]entry[V]
  ttl  time.Duration
}

func New[K comparable, V any](ttl time.Duration) Cache[K, V] {
  return Cache[K, V]{data: make(map[K]entry[V]), ttl: ttl}
}

func (c Cache[K, V]) Set(key K, value V) {
  c.data[key] = entry[V]{value: value, exp: time.Now().Add(c.ttl)}
}

func (c Cache[K, V]) Get(key K) (V, bool) {
  e, ok := c.data[key]
  if !ok {
    var zero V
    return zero, false
  }
  if time.Now().After(e.exp) {
    delete(c.data, key)
  }
  return e.value, true
}

func main() {
  c := New[string, int](50 * time.Millisecond)
  c.Set("a", 1)

  v, ok := c.Get("a")
  fmt.Println("immediately:", v, ok)

  time.Sleep(100 * time.Millisecond)
  v, ok = c.Get("a")
  fmt.Println("after ttl:", v, ok)

  for i := 0; i < 5; i++ {
    c.Set(fmt.Sprintf("k%d", i), i)
  }
  time.Sleep(100 * time.Millisecond)
  fmt.Println("entries still in map after all expired, none re-Get:", len(c.data))

  c2 := c
  c2.Set("shared", 42)
  sharedVal, sharedOK := c.Get("shared")
  fmt.Println("c (unrelated-looking copy) also sees c2's write:", sharedVal, sharedOK)
}

หา bug

c.Get("a") หลังจาก TTL ผ่านไปแล้วยังรายงาน true อยู่ ตัวแปร cache สองตัวที่ดูเหมือนเป็นอิสระจากกันกลับเห็นการเขียนของกันและกัน ลองระบุ bug อย่างน้อย 3 ใน 4 ตัวก่อนเปิดเฉลย

เฉลย 4 bug

Get ลบ entry ที่หมดอายุแล้ว แต่ยัง return มันเหมือนยัง valid อยู่ if time.Now().After(e.exp) { delete(c.data, key) } รัน แล้ว execution ก็ตกไปที่ return e.value, true ที่ไม่มีเงื่อนไขด้านล่างตรงๆ — ไม่มี return zero, false บน path ที่หมดอายุเลย output จริง: after ttl: 1 true ทั้งที่ควรเป็น 0 false entry ถูกลบออกจาก map ถูกต้องด้วยซ้ำ (delete บน map ที่แชร์ผ่าน reference ทำงานได้ปกติแม้ผ่าน value receiver) — แต่ค่าเก่ายังถูกส่งออกไปอยู่ดี
ทั้ง Set และ Get ไม่ sync การเข้าถึง map เลย ไม่มี sync.Mutex ที่ไหนใน Cache เลย ลองรัน 4 goroutine เรียก Set/Get บน cache เดียวกันพร้อมกัน แล้ว runtime ของ Go เอง — ไม่ใช่แค่ race detector — จับได้ตรงๆ:
```
fatal error: concurrent map writes
```
นี่ไม่ใช่ finding ที่ต้องพึ่ง -race เท่านั้น การเขียน map พร้อมกันแบบไม่ sync ทำให้ process crash ทันที ไม่ว่าจะมี race detector ติดอยู่หรือไม่ (-race เพิ่มเติมจะรายงาน goroutine และบรรทัดที่เกี่ยวข้องแบบเป๊ะๆ)
entry ที่หมดอายุไม่เคยถูกลบเลย นอกจากบังเอิญมีใครเรียก Get บน key นั้นซ้ำ demo นี้ตั้ง 5 key ปล่อยให้หมดอายุทั้งหมด แล้วไม่แตะมันด้วย Get อีกเลย — len(c.data) รายงานว่ายังเหลือทั้ง 5 ตัว cache ที่ใช้แบบ Set เป็นหลัก (เขียนค่า แล้วแทบไม่เคยหรือไม่เคย lookup key นั้นซ้ำอีกเลย) จะโตไม่มีที่สิ้นสุด ไม่มีอะไรมากวาดมันเลย
New คืน Cache[K, V] แบบ value พร้อม method แบบ value receiver การ assign ธรรมดาจึงกลาย alias แทนที่จะเป็นการ copy แบบเงียบๆ c2 := c ดูเหมือน copy ที่เป็นอิสระ — Cache เป็นแค่ struct ธรรมดา ท้ายที่สุดแล้ว — แต่ state จริงของ Cache มีแค่ map เดียว และ map เป็น reference type c2.Set("shared", 42) มองเห็นได้ผ่าน c ด้วย: c (unrelated-looking copy) also sees c2's write: 42 true ไม่มีอะไรผิดกฎ Go ตรงนี้เลยในทางเทคนิค — มันคือ value type ที่ “value” semantics ของมันหลอกตาเพราะสิ่งที่มันบรรจุอยู่ข้างใน

The corrected program

Open in Playground

package main

import (
  "fmt"
  "sync"
  "time"
)

type entry[V any] struct {
  value V
  exp   time.Time
}

type Cache[K comparable, V any] struct {
  mu   sync.Mutex // fixed: guards data against concurrent Set/Get
  data map[K]entry[V]
  ttl  time.Duration
  done chan struct{}
}

func New[K comparable, V any](ttl time.Duration) *Cache[K, V] { // fixed: pointer — copies now share explicitly, on purpose
  c := &Cache[K, V]{data: make(map[K]entry[V]), ttl: ttl, done: make(chan struct{})}
  go c.janitor(ttl) // fixed: proactively evict expired entries, not just on access
  return c
}

func (c *Cache[K, V]) janitor(interval time.Duration) {
  t := time.NewTicker(interval)
  defer t.Stop()
  for {
    select {
    case <-t.C:
      c.mu.Lock()
      for k, e := range c.data {
        if time.Now().After(e.exp) {
          delete(c.data, k)
        }
      }
      c.mu.Unlock()
    case <-c.done:
      return
    }
  }
}

func (c *Cache[K, V]) Close() { close(c.done) } // fixed: caller can stop the janitor goroutine

func (c *Cache[K, V]) Set(key K, value V) {
  c.mu.Lock()
  defer c.mu.Unlock()
  c.data[key] = entry[V]{value: value, exp: time.Now().Add(c.ttl)}
}

func (c *Cache[K, V]) Get(key K) (V, bool) {
  c.mu.Lock()
  defer c.mu.Unlock()
  e, ok := c.data[key]
  if !ok || time.Now().After(e.exp) { // fixed: expired counts as not found
    var zero V
    return zero, false
  }
  return e.value, true
}

func main() {
  c := New[string, int](50 * time.Millisecond)
  defer c.Close()
  c.Set("a", 1)

  v, ok := c.Get("a")
  fmt.Println("immediately:", v, ok)

  time.Sleep(100 * time.Millisecond)
  v, ok = c.Get("a")
  fmt.Println("after ttl:", v, ok)

  for i := 0; i < 5; i++ {
    c.Set(fmt.Sprintf("k%d", i), i)
  }
  time.Sleep(200 * time.Millisecond) // give the janitor a couple of ticks
  fmt.Println("entries left after janitor sweep:", len(c.data))

  c2 := c // fixed: this is now an explicit pointer copy — no surprise aliasing of a value type
  c2.Set("shared", 42)
  sharedVal, sharedOK := c.Get("shared")
  fmt.Println("c and c2 are the same *Cache on purpose:", sharedVal, sharedOK)
}

รันเวอร์ชันที่แก้แล้ว: after ttl: 0 false, entries left after janitor sweep: 0 (background goroutine janitor ที่หยุดได้ผ่าน Close กวาด entry ที่หมดอายุตาม ticker) และ c and c2 are the same *Cache on purpose: 42 true — การ alias ยังเหมือนเดิม แต่ไม่ใช่เรื่องเซอร์ไพรส์เงียบๆ อีกต่อไป: *Cache[K, V] บอกไว้ในตัว type เองแล้วว่า copy จะแชร์ state กัน เหมือน pointer ตัวอื่นทุกตัวใน Go เทส stress เวอร์ชันนี้ด้วย 4 goroutine เรียก Set/Get พร้อมกันภายใต้ go run -race ไม่รายงาน race และไม่มี fatal error เลย — เวอร์ชันด้านบน crash ด้วย fatal error: concurrent map writes ภายใต้ load แบบเดียวกัน

🟦 generic type ไม่ได้ทำให้ปลอดภัยขึ้นอัตโนมัติ

ทุก bug ที่นี่จะมีอยู่เหมือนเดิมทุกประการใน cache ที่ใช้ map[string]int ธรรมดาไม่มี generic — generics ไม่ได้เปลี่ยนอะไรเลยเรื่องความจำเป็นต้องมี mutex, expiry check ที่ถูกต้อง, หรือ pointer semantics อย่าให้ความแปลกใหม่ของ type parameter ทำให้เสียสมาธิจาก review checklist เดียวกันที่ใช้กับ stateful type ตัวไหนก็ได้ ดู Generics และ The sync package