fix(waf): call WAF.Close()

[M4tteoP]

Starting from Coraza v3.5.0, regex memoization is enabled by default:

Memoization is enabled by default and uses a global cache to reuse compiled patterns across WAF instances, reducing memory consumption and startup overhead. In long-lived processes that perform live reloads, use WAF.Close() (via experimental.WAFCloser) to release cached entries when a WAF is destroyed

Expecially considering the added possibility of having per-route WAFs, this is a useful feature in our context but requires proper handling to avoid memory leaks. This PR addresses this by tracking all WAF instances (main and per-route) in a shared registry, and releasing their memoized regex cache entries when the factory is destroyed via OnDestroy.

I asked to Claude to write a test that proves it and looks reasonable to me, but I would love @wbpcode review, expecially in the context of per-route WAF lifecycles.

Before these changes:

--- FAIL: Test_OnDestroyReleasesMemory (0.09s)
    /Repo/built-on-envoy/extensions/composer/waf/waf_test.go:1747: 
        	Error Trace:	/Repo/built-on-envoy/extensions/composer/waf/waf_test.go:1747
        	Error:      	"11320" is not less than or equal to "1024"
        	Test:       	Test_OnDestroyReleasesMemory
        	Messages:   	heap grew 11320 KB after 30 reloads, expected ≤ 1024 KB: OnDestroy must call Close() on all WAF instances to release Coraza's memoized regex cache
FAIL
FAIL	github.com/tetratelabs/built-on-envoy/extensions/composer/waf	0.658s
FAIL

Assisted by Claude 🤖

[wbpcode]

Thanks for this update.

I wonder when the Close() is called, will all cache entry be cleared or only the cache entry related to the WAF instance will be cleared? If the answer is latter? could this method be called in any thread without thread safe problem?

Or we have no clear idea for the above problems temporarily, I inclined to do nothing first and keep the global cache first.

[wbpcode]

per route level waf will have completely independent lifetime with waf configuration in factory. So, I guess it's may not be the correct pattern to call Close() method of route level WAF when the OnDesotry() is called.

And when the configuration is updating, the new filter factory will be created first, then the OnDestory() of old filter factory will be called. That's say the current solution will also call the Close() of new created WAF. I am not sure this is the correct behavior.

[nacx]

@M4tteoP @wbpcode what is the path forward to this pull request?

[M4tteoP]

when the Close() is called, will all cache entry be cleared or only the cache entry related to the WAF instance will be cleared?

Only the entries belonging to that WAF instance and that they have no other owners. Each entry has a list of owners, and the release deletes the entry only if no owners are left.

And when the configuration is updating, the new filter factory will be created first, then the OnDestory() of old filter factory will be called.

So this should work: the new WAF is created and takes ownership of all the required entries, then the old one is deleted, and only the entries that are no longer owned will be freed.

If the answer is latter? could this method be called in any thread without thread safe problem?

The cache is a sync.Map, and each entry has its own mutex. As far as I can tell we should be safe.

per route level waf will have completely independent lifetime with waf configuration in factory. So, I guess it's may not be the correct pattern to call Close() method of route level WAF when the OnDesotry() is called.

Based on this, I think we should be good on calling the Close() for the main WAF when OnDestroy() is called, but it is wrong to tie that OnDestroy() with route level waf instances. Could you please elaborate on route level waf lifetime? Is there a per-route OnDestroy() where we can trigger Close() of the individual per-route waf? @wbpcode