[GitHub] [lucenenet] Shazwazza opened a new issue #478: Disposing IndexWriter doesn't check if it's already disposed/closed

[GitBox <gi...@apache.org>]

Shazwazza opened a new issue #478:
URL: https://github.com/apache/lucenenet/issues/478


   I'm (thankfully finally) porting my Examine project over to 4.8 🎉  I've noticed a peculiar thing... If you Dispose (and therefore Close) the IndexWriter, most properties accessed like `writer.Analyzer` will throw an `ObjectDisposedException`. But if you Dispose 2x on the IndexWriter itself, this does not occur, it just ignores the 2nd (and more times). It also doesn't occur if you call `IndexWriter.Dispose(bool waitForMerges)` after its already disposed, it just ignores it.
   
   Just seems a little inconsistent is all. I had a look at the latest Java source and seems like it will do the same thing. I just wanted to post this here to see what anyone thinks. I guess since we want to stay as close to the java source as possible it will prob remain this way but let me know what you think.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [lucenenet] Shazwazza commented on issue #478: Disposing IndexWriter doesn't check if it's already disposed/closed

[GitBox <gi...@apache.org>]

Shazwazza commented on issue #478:
URL: https://github.com/apache/lucenenet/issues/478#issuecomment-830563891


   The main reason I log this is really in case people go searching for a similar issue. The issue that was raised when porting over my work was a change in code in Lucene which revealed a mistake in my code. 
   
   I had accidentally disposed the IndexWriter but then tried to 
   
   ```cs
   IndexWriter?.Analyzer.Dispose();
   ```
   
   which threw the ObjectDisposedException. In my own indexer class, when it disposes at the end of being used (cannot be re-used) it disposes all other things that are disposable. The analyzer was one of them but due to the ordering of things I got the exception. The mistake in my code is disposing an object of an object that was originally owned by the parent. 
   
   I guess because Analyzer is virtual, it could do more than just return the value from the ctor so safer to throw. All good, will close this just wanted to ask. Cheers!
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [lucenenet] NightOwl888 commented on issue #478: Disposing IndexWriter doesn't check if it's already disposed/closed

[GitBox <gi...@apache.org>]

NightOwl888 commented on issue #478:
URL: https://github.com/apache/lucenenet/issues/478#issuecomment-830537225


   Okay, what alternative would you expect? To be able to reopen? To throw an exception?
   
   Per the [IDisposable documentation](https://docs.microsoft.com/en-us/dotnet/api/system.idisposable.dispose?redirectedfrom=MSDN&view=net-5.0#remarks):
   
   > If an object's [Dispose](https://docs.microsoft.com/en-us/dotnet/api/system.idisposable.dispose) method is called more than once, the object must ignore all calls after the first one. The object must not throw an exception if its [Dispose](https://docs.microsoft.com/en-us/dotnet/api/system.idisposable.dispose) method is called multiple times. Instance methods other than [Dispose](https://docs.microsoft.com/en-us/dotnet/api/system.idisposable.dispose) can throw an [ObjectDisposedException](https://docs.microsoft.com/en-us/dotnet/api/system.objectdisposedexception) when resources are already disposed.
   
   Note it says "must", not "should". Also, since an `IndexWriter` cannot be reopened after it is disposed, it seems like this is the right approach.
   
   Do keep in mind that using the new `using` statement from C# 8.0, `Dispose` may be called automatically by the compiler even after it had already been called explicitly.
   
   ```c#
   { // Block for scope
       using var x = new MyDisposable();
       // Do some work (there may be an exception)
       x.Dispose();
       // Do some more work
   } // Dispose is called here whether the above Dispose() gets called or not
   ```
   
   This is a common pattern when using `IndexWriter.GetReader()`, which must be called before disposing the `IndexWriter`.
   
   ```c#
   using IndexWriter writer = new IndexWriter(directory, config);
   // Add some docs
   using IndexReader reader = writer.GetReader();
   writer.Dispose();
   // Use the reader...
   ```
   
   It would be awkward to work with if there were an exception thrown just because we want to use a `using` statement, since the purpose is to ensure `Dispose` is called *at least* once regardless of whether an exception occurs or the operation is successful.
   
   As for the overload with `waitForMerges`, it conflicts with the protected `Dispose(bool)` overload that should exist in .NET for subclasses to implement. I am considering to rename it `DisposeNoWait()` and add an extension method to patch the current calls to the public `Dispose(bool)` to call either `Dispose()` or `DisposeNoWait()` depending on the `bool` value.
   
   What use case are you trying to solve for that you expect this to work differently?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [lucenenet] Shazwazza closed issue #478: Disposing IndexWriter doesn't check if it's already disposed/closed

[GitBox <gi...@apache.org>]

Shazwazza closed issue #478:
URL: https://github.com/apache/lucenenet/issues/478


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org