[GitHub] [lucenenet] ShreyTiwari opened a new issue #615: File Handle Leak

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

ShreyTiwari opened a new issue #615:
URL: https://github.com/apache/lucenenet/issues/615


   Hi,
   
   I am new to the Lucenenet project. While investigating this project, I found this bug related to a file handle leak.
   **Location**: https://github.com/apache/lucenenet/blob/master/src/Lucene.Net.Demo/SearchFiles.cs#L117
   **Bug**: The "input" variable can point to a FileStream that is never disposed.
   
   Another example:
   **Location**: https://github.com/apache/lucenenet/blob/master/src/Lucene.Net.Tests.Suggest/Suggest/Fst/LargeInputFST.cs#L45
   **Bug**: The "reader" variable points to a FileStream that is never disposed.
   
   I was also wondering if such kind of leaks related to IDisposables exist in the code base. Running an existing [CodeQL](https://codeql.github.com/docs/codeql-language-guides/codeql-for-csharp/) (static analysis tool) query for detecting local un-disposed objects in the code, I observed 45 alerts being generated for the entire codebase. I do not know if all of these 45 alerts are actual leaks or benign alerts.
   
   Could you please share information on ways in which leaks, in general, are currently being prevented in the Lucenenet project (any tools being used or processes in place)?
   
   Thanks in advance!
   
   


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org

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

[GitHub] [lucenenet] NightOwl888 commented on issue #615: File Handle Leak

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

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


   Thanks for the report.
   
   Technically, this is a duplicate of #265. We haven't yet reviewed the `IDisposable` usages to ensure they are correct from a .NET perspective. Although, that issue is up-for-grabs if you are interested in contributing, and **we welcome your assistance**. We have largely saved this task for the community and have been focusing on more complicated issues that require more than just an understanding of .NET best practices to solve.
   
   Lucene.NET is (for the most part) a line-by-line port of [Lucene 4.8.0](https://github.com/apache/lucene/tree/releases/lucene-solr%2F4.8.0/lucene). As such, we carried over many of the gaps between Java and .NET. One such gap is that in Java the [Closable interface](https://docs.oracle.com/javase/8/docs/api/java/io/Closeable.html) inherits the [AutoCloseable interface](https://docs.oracle.com/javase/8/docs/api/java/lang/AutoCloseable.html), which can be configured to automatically be cleaned up. We haven't yet researched whether Lucene is configured with that option enabled, but suspect that is the reason why the code doesn't explicitly call `.close()` (and why the .NET version doesn't explicitly call `.Dispose()`). This includes the two [you mentioned](https://github.com/apache/lucenenet/issues/615#issue-1122670124).
   
   Given this fact, there are likely lots of places where there are missing calls to `Dispose()`, most of which can be added (with a comment `LUCENENET specific: added call to Dispose()` or `LUCENENET specific: added using block`).
   
   Note there is at least 1 known place where the call to `Dispose()` is problematic in #271. We are seeing multiple calls to `Dispose()` because we have added [this line in `Tokenizer`](https://github.com/apache/lucenenet/blob/972d1f50dcb7b49cd08ae91cd984309f942909b8/src/Lucene.Net/Analysis/Tokenizer.cs#L76) that [didn't exist in Lucene](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.0/lucene/core/src/java/org/apache/lucene/analysis/Tokenizer.java#L65-L70). I suspect that Lucene falls back to the `AutoCloseable` config in this case, and the closest .NET equivalent is to use a [finalizer](https://techcommunity.microsoft.com/t5/iis-support-blog/understanding-when-to-use-a-finalizer-in-your-net-class/ba-p/347760), but both of these assumptions need confirmation. Clearly, using a finalizer has drawbacks and we should avoid that approach and use explicit calls to `Dispose()`, if possible. It also seems like `Dispose()` and `Close()` should be separate concepts in this par
 ticular case, being that this class is expected to be reused after it is "closed" without creating a new instance. But, the bottom line is we cannot just add all of the missing `Dispose()` calls and `using` statements and assume that is the correct approach - each case needs to be considered carefully according to its existing usage in Java.
   
   It sounds like [CodeQL](https://codeql.github.com/docs/codeql-language-guides/codeql-for-csharp/) can be utilized to speed up the process of locating them. My suggestion would be to submit a separate commit for each of the issues you found, and break them down into multiple PRs in some sensible way, so we can review and test them thoroughly chunk by chunk
   
   


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org

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

[GitHub] [lucenenet] ShreyTiwari commented on issue #615: File Handle Leak

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

ShreyTiwari commented on issue #615:
URL: https://github.com/apache/lucenenet/issues/615#issuecomment-1030885712


   Thank you for all the useful links. From the current analysis it doesn't seem like all the alerts that have been raised could be bugs. For example, "MemoryStream" instances in the code do not necessarily have to be disposed since they would be garbage collected without causing any resource leaks.
   
   I will raise PR's for the bugs and submit the fixes as part of separate commits. 


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org

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