Open Bug 1643313 Opened 4 years ago

Consider having separate subclasses for DirectoryLockImpl to differentiate whether mOpenListener is used


(Core :: Storage: Quota Manager, enhancement, P3)





(Reporter: tt, Unassigned)


See discussion in:

mOpenListener is always set when aInternal is false in CTOR. It can only be set in CTOR so I make it as InitializedOnce in Bug 1641231.

However, the consumers for InitializedOnce are often set (thus, it often used in these two patterns: InitializedOnce<const NotNull<T>> or LazyInitializedOnceEarlyDestructible<const NotNull<T>>)

Here, it can be a nullptr for the whole life of DirectoryLockImpl. My understanding is that it can only be a nullptr for eviction (for QM) or database maintenance (for IDB).

Meanwhile, there is a member function DirectoryLockImpl::NotifyOpenListener() which should only be called when it's not in these two cases. There is a debug assertion to avoid misuse, but still it's not 100% obvious from the code.

Simon suggested at least we should move the assertion to the call sites and add comments around there to assert/explain why we are sure mOpenListener is valid or have seperate subclasses or add a template argument for this distinction to make it safer and exclude the possibility of nullptr dereferences.

You need to log in before you can comment on or make changes to this bug.