Open jj683 opened 1 month ago
Hmmmm interesting.
I think my reasoning here is as follows:
The unsafe free list is assumed to be used in a single threaded program. Therefore the destruction of the heap means the end of the program lifetime. Therefore clearing is just unnecessary work. Note that at this level of abstraction these lists hold plain memory with no destructor or no further nested resources so it is safe and not a leak to not return them to the language runtime heap (this may not be 100% true if the heap is composed with another heap that falsifies this statement, none exist however in the Immer codebase at the moment though).
In the thread local list case, however, the thread may end before the program ends. Therefore we should return the blocks to the previous heap to avoid leaks and so that it can make use of those in any useful way.
With these assumptions the current code was correct. But maybe you were using the code in some way that negates some of the assumptions? What prompted you to make these changes?
We encountered different behavior of thread_local_free_list_heap and unsafe_free_list_heap data structures even when used in the single thread environment.
Unfortunately I am still in the process of building a small test to reproduce it, but while reading the code I noticed that the clear function is not called for unsafe_free_list_storage similarly to the thread_local_free_list_storage container. Is this expected behavior?