Contrarily to the design I proposed in #84, this pull request takes a step back and moves ownership of fs::File down into Buffer so that buffers can be pruned more easily from Project when nobody is using them anymore.
My original rationale for storing fs::File (and a bi-directional mapping between files and buffers) in Project was that it felt awkward to clutter the buffer with more responsibilities than it already had. However, the key realization I had today was that Project isn't the actual owner of buffers at all. For example, when the last BufferView holding a particular buffer goes away, we want that buffer to be forgotten in Project as well. The same reasoning applies when all buffer::rpc::Service instances are dropped. This clearly indicated that the project should hold weak references to buffers instead.
The question still applies though: why moving the ownership of fs::File down into the buffer? The answer has to do with how we prune buffers from the project. Indeed, turning buffer into a Weak inside of Project allows it to let go of the buffer when the last real owner gets dropped. However, the same would not be true for fs::File if it lived inside of project. When presented with this challenge, I considered two choices:
Find a way of signalling Project when dropping a buffer, so that the project could let go of all the state associated with that particular buffer. Circular references are always tricky to reason about, so this didn't really feel like a good solution.
Move fs::File into Buffer and let the vector holding the list of weak references to buffers leak pointers for those buffers that are dropped.
I ended up going with the second approach, so that when the buffer is dropped by its last actual owner, it will also let go of the file (and its associated descriptor). All these details are cleanly abstracted away by the BufferWeakSet struct, which, when queried, cleans up all the weak references that are dangling.
Even though we will end up leaking a few pointers, this feels like the most elegant solution (among the ones I considered) given the set of constraints presented above.
Contrarily to the design I proposed in #84, this pull request takes a step back and moves ownership of
fs::Filedown intoBufferso that buffers can be pruned more easily fromProjectwhen nobody is using them anymore.My original rationale for storing
fs::File(and a bi-directional mapping between files and buffers) inProjectwas that it felt awkward to clutter the buffer with more responsibilities than it already had. However, the key realization I had today was thatProjectisn't the actual owner of buffers at all. For example, when the lastBufferViewholding a particular buffer goes away, we want that buffer to be forgotten inProjectas well. The same reasoning applies when allbuffer::rpc::Serviceinstances are dropped. This clearly indicated that the project should hold weak references to buffers instead.The question still applies though: why moving the ownership of
fs::Filedown into the buffer? The answer has to do with how we prune buffers from the project. Indeed, turning buffer into aWeakinside ofProjectallows it to let go of the buffer when the last real owner gets dropped. However, the same would not be true forfs::Fileif it lived inside of project. When presented with this challenge, I considered two choices:Projectwhen dropping a buffer, so that the project could let go of all the state associated with that particular buffer. Circular references are always tricky to reason about, so this didn't really feel like a good solution.fs::FileintoBufferand let the vector holding the list of weak references to buffers leak pointers for those buffers that are dropped.I ended up going with the second approach, so that when the buffer is dropped by its last actual owner, it will also let go of the file (and its associated descriptor). All these details are cleanly abstracted away by the
BufferWeakSetstruct, which, when queried, cleans up all the weak references that are dangling.Even though we will end up leaking a few pointers, this feels like the most elegant solution (among the ones I considered) given the set of constraints presented above.