Ceph » CephFS

The issue suggests spreading LogSegment* as shared_ptr while class MDLog manages those ptrs lifetime (creates/stores/deletes) and the other objects use get_current_segment() / get_segment() for reading it.
So Im not sure what the benefit of sharing LogSegment* ownership via shared_ptr as only one owner responsible to create/delete them.
Am I missing something :-)

Tamar Shacked wrote:

The issue suggests spreading LogSegment* as shared_ptr while class MDLog manages those ptrs lifetime (creates/stores/deletes) and the other objects use get_current_segment() / get_segment() for reading it.
So Im not sure what the benefit of sharing LogSegment* ownership via shared_ptr as only one owner responsible to create/delete them.
Am I missing something :-)

Yeah, IMO it should be a good habit to use the shared_ptr to avoid potential use-after-free bugs as we hit in client/Client.cc before, which were very hard to debug. And this could simplify the code too.

Yeah, IMO it should be a good habit to use the shared_ptr to avoid potential use-after-free bugs as we hit in client/Client.cc before, which were very hard to debug. And this could simplify the code too.

Sure, I thought that a case of the current_segment being deleted while it's getting read isn't applicable, but using shared_ptr is safer and I"ll prepare the PR.

Still want to note that shared_ptr can be expansive:
https://www.modernescpp.com/index.php/memory-and-performance-overhead-of-smart-pointer

Was a memory pool which can be grow and shrink was considered (or maybe it isn't not a good choice as the LogSegments holds a few list<T*>)?

For those following along, most MDS operations involve something like "mut->ls = get_current_segment()", and the possibility of breaking those references is the concern this ticket is meant to address.