`IndexWriter.Dispose(bool disposing, bool waitForMerges)` breaks the standard `Dispose(bool disposing)` contract
- Dominant language
- C#
- Stars
- 2.4k
- Forks
- 658
- Avg merge
- 3d 5h
- Merged PRs (30d)
- 9
Description
> Just brainstorming here. This contract has always been an issue because the analyzers warn if the `protected virtual void Dispose(bool disposing)` signature is not used. But until now it was clear that this contract was required to pass the `waitForMerges` state through to subclasses. Is there a reasonable way we can remove this parameter and restore the original signature before the release, or are we stuck because `IndexWriter` still needs this to be passed through?
> In other words, can we keep `waitForMerges` out of the public API that inheritors of this class use?
_Originally posted by @NightOwl888 in https://github.com/apache/lucenenet/pull/1311#discussion_r3488376865_
`IndexWriter` exposes its dispose hook as `protected virtual void Dispose(bool disposing, bool waitForMerges)` instead of the conventional `protected virtual void Dispose(bool disposing)`, a LuceneNet-specific choice made so the public `Dispose()` and `[Obsolete] Dispose(bool waitForMerges)` overloads could thread `waitForMerges` down to the close path. This diverges from the dispose pattern the analyzers expect (hence the several `[SuppressMessage]` suppressions on the method) and, since it's a `protected virtual` member, changing it becomes a breaking change once we ship, so it should be fixed before the next release. We can restore the standard `Dispose(bool disposing)` signature and keep `waitForMerges` off the inheritance surface, since both public call sites already know the flag and can pass it into the internal shutdown path directly rather than through the virtual hook (the only tradeoff, that a subclass can't influence the `waitForMerges: false` shutdown, is on the `[Obsolete]` path; upstream Java did allow overriding `close(boolean)`, but no in-tree subclass relies on that today). Note that the `waitForMerges` support was removed in Lucene 5.0.
Contributor guide
Assessment
This issue has not been assessed yet.