You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I have also been made aware that there is at least one class (Lucene.Net.Store.Lock, if I recall correctly) that is designed to be re-opened after it is closed.
In investigating the Lock class, I found that in Lucene 5.3.0, the Lock API was simplified to remove the ability to re-obtain a lock using the same instance, and instead it only provides a method to close (dispose) the lock once you're done with it. In that version of Lucene, it seems like IDisposable is the correct choice. I wish we could mark all these other methods as Obsolete, but unfortunately Obtain et al. are how it is intended to be used today, so we should not be throwing warnings upon use, even though the methods will be removed in a future version of Lucene.NET.
In 4.8, it certainly does seem like this type is designed to be reusable, as you could theoretically obtain the lock when you need it, close() it to release it, and re-obtain it to do something else. This indicates that we should update Lock to use the new ICloseable interface once #271 is merged. To encourage best practices and compatibility with future Lucene.NET, we should leave this as IDisposable where Dispose calls Close, but provide this additional interface to allow for reuse. To avoid this being a breaking change, we should not throw if Dispose is called more than once, nor make it a no-op in that case. It can simply delegate the call to Close.
If possible, we should create an analyzer that looks for calls to any Lock methods after Dispose is called, and warn against such use.
The text was updated successfully, but these errors were encountered:
Is there an existing issue for this?
Task description
From investigating #265:
In investigating the Lock class, I found that in Lucene 5.3.0, the Lock API was simplified to remove the ability to re-obtain a lock using the same instance, and instead it only provides a method to close (dispose) the lock once you're done with it. In that version of Lucene, it seems like IDisposable is the correct choice. I wish we could mark all these other methods as Obsolete, but unfortunately Obtain et al. are how it is intended to be used today, so we should not be throwing warnings upon use, even though the methods will be removed in a future version of Lucene.NET.
In 4.8, it certainly does seem like this type is designed to be reusable, as you could theoretically obtain the lock when you need it,
close()
it to release it, and re-obtain it to do something else. This indicates that we should update Lock to use the newICloseable
interface once #271 is merged. To encourage best practices and compatibility with future Lucene.NET, we should leave this as IDisposable where Dispose calls Close, but provide this additional interface to allow for reuse. To avoid this being a breaking change, we should not throw if Dispose is called more than once, nor make it a no-op in that case. It can simply delegate the call to Close.If possible, we should create an analyzer that looks for calls to any Lock methods after Dispose is called, and warn against such use.
The text was updated successfully, but these errors were encountered: