Skip to content

fw: drop evicted entries from CsLRU location index - #213

Open
Harsh23Kashyap wants to merge 1 commit into
named-data:mainfrom
Harsh23Kashyap:fix/cs-lru-locations-leak
Open

Harsh23Kashyap wants to merge 1 commit into
named-data:mainfrom
Harsh23Kashyap:fix/cs-lru-locations-leak

Conversation

@Harsh23Kashyap

Copy link
Copy Markdown

Fixes #210.

Summary

CsLRU.EvictEntries removed the evicted entry from the LRU queue but never deleted its index from the locations map. The erase goes through eraseCsDataFromReplacementStrategy, which does not call BeforeErase, so no other path cleans it up. A forwarder running at content-store capacity leaks one map entry and one orphaned list element per eviction, growing the index without bound for the life of the process.

The fix deletes the evicted index from locations in EvictEntries. This is separate from #154, which tracks buffer allocations in the face link service.

Repro evidence

Go 1.26.6, linux/amd64, ndnd main @ d51682b. CS capacity 5, 100 distinct Data packets inserted.

Regression test against unpatched main:

$ go test ./fw/table/ -run TestCsLRUEvictionForgetsLocations -v
=== RUN   TestCsLRUEvictionForgetsLocations
    cs-lru_test.go:39:
        	Error:      	Not equal:
        	            	expected: 5
        	            	actual  : 100
        	Messages:   	CsLRU.locations keeps stale entries after eviction (memory leak)
--- FAIL: TestCsLRUEvictionForgetsLocations (0.00s)
FAIL
FAIL	github.com/named-data/ndnd/fw/table	0.004s

Same test against this branch:

$ go test ./fw/table/ -run TestCsLRUEvictionForgetsLocations -v
=== RUN   TestCsLRUEvictionForgetsLocations
--- PASS: TestCsLRUEvictionForgetsLocations (0.00s)
PASS
ok  	github.com/named-data/ndnd/fw/table	0.004s

Testing

  • New regression test TestCsLRUEvictionForgetsLocations: fails before the fix, passes after.
  • go test ./... - all 22 packages pass.
  • go vet ./fw/... and gofmt clean.

CsLRU.EvictEntries removed the evicted entry from its queue but left the
entry in the locations map, because the CS-side erase callback bypasses
BeforeErase. Every eviction leaked one map entry and one list element, so
a forwarder running at CS capacity grew the index without bound.

Delete the evicted index from locations on eviction. Add a regression test
that inserts 100 entries at capacity 5 and checks the index shrinks with
the queue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fw: CsLRU grows its locations map without bound on every eviction

1 participant