Skip to content

Commit

Permalink
Finalizer fix (apache#951)
Browse files Browse the repository at this point in the history
* Fixed performance / cpu resource leak  issue of IndexReader

Dropped outdated sdk build dependencies

* Reverted changes to get it on my machine publishing nuget packages

* Fixed typo

* Review findings

* Update Lucene.Net.Tests.CodeAnalysis.csproj

* publish-test-results-for-test-projects.yml: Updated Lucene.Net.Tests.CodeAnalysis to publish tests for net8.0

* .github/workflows/Lucene-Net-Tests-CodeAnalysis.yml: Updated to test on net8.0

---------

Co-authored-by: Shad Storhaug <shad@shadstorhaug.com>
  • Loading branch information
stesee and NightOwl888 authored Aug 15, 2024
1 parent 09a6de9 commit 1854672
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ steps:
- template: publish-test-results.yml
parameters:
testProjectName: 'Lucene.Net.Tests.CodeAnalysis'
framework: 'net5.0' # Since condtions are not supported for templates, we check for the file existence within publish-test-results.yml
framework: 'net8.0' # Since condtions are not supported for templates, we check for the file existence within publish-test-results.yml
vsTestPlatform: '${{ parameters.vsTestPlatform }}'
osName: '${{ parameters.osName }}'
testResultsFormat: '${{ parameters.testResultsFormat }}'
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/Lucene-Net-Tests-CodeAnalysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ jobs:
fail-fast: false
matrix:
os: [windows-latest, ubuntu-latest]
framework: [net5.0]
framework: [net8.0]
platform: [x64]
configuration: [Release]
exclude:
Expand Down
8 changes: 4 additions & 4 deletions Lucene.Net.sln
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio Version 16
# Visual Studio Version 17
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
Expand All @@ -16,7 +16,7 @@ Microsoft Visual Studio Solution File, Format Version 12.00
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
VisualStudioVersion = 16.0.29806.167
VisualStudioVersion = 17.10.35004.147
MinimumVisualStudioVersion = 15.0.26730.8
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "azure-templates", "azure-templates", "{05CE3A39-40D4-452D-AFE0-E57E536A08C6}"
ProjectSection(SolutionItems) = preProject
Expand All @@ -41,10 +41,10 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = ".build", ".build", "{4016BD
build = build
build.bat = build.bat
build.ps1 = build.ps1
.build\runbuild.ps1 = .build\runbuild.ps1
.build\dependencies.props = .build\dependencies.props
.build\nuget.props = .build\nuget.props
.build\release.targets = .build\release.targets
.build\runbuild.ps1 = .build\runbuild.ps1
.build\TestReferences.Common.targets = .build\TestReferences.Common.targets
TestTargetFramework.props = TestTargetFramework.props
EndProjectSection
Expand Down Expand Up @@ -263,7 +263,7 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Lucene.Net.Tests.AllProject
EndProject
Project("{E24C65DC-7377-472B-9ABA-BC803B73C61A}") = "websites", "websites\", "{8988CDA4-8420-4BEE-869A-66825055EED2}"
ProjectSection(WebsiteProperties) = preProject
TargetFrameworkMoniker = ".NETFramework,Version%3Dv4.6.2"
TargetFrameworkMoniker = ".NETFramework,Version%3Dv4.8"
Debug.AspNetCompiler.VirtualPath = "/localhost_59352"
Debug.AspNetCompiler.PhysicalPath = "websites\"
Debug.AspNetCompiler.TargetPath = "PrecompiledWeb\localhost_59352\"
Expand Down
10 changes: 3 additions & 7 deletions src/Lucene.Net/Index/IndexReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
using Lucene.Net.Support;
using Lucene.Net.Support.Threading;
using Lucene.Net.Util;

#if !FEATURE_CONDITIONALWEAKTABLE_ENUMERATOR
using Lucene.Net.Util.Events;
#endif

using System;
using System.Collections;
using System.Collections.Generic;
Expand Down Expand Up @@ -655,12 +657,6 @@ internal void SubscribeToGetCacheKeysEvent(WeakEvents.GetCacheKeysEvent getCache
getCacheKeysEvent.Subscribe(OnGetCacheKeys);
}

// LUCENENET specific: Clean up the weak event handler if this class goes out of scope
~IndexReader()
{
Dispose(false);
}

// LUCENENET specific: Add weak event handler for .NET Standard 2.0 and .NET Framework, since we don't have an enumerator to use
private void OnGetParentReaders(WeakEvents.GetParentReadersEventArgs e)
{
Expand Down Expand Up @@ -781,4 +777,4 @@ public interface IReaderDisposedListener
/// Invoked when the <see cref="IndexReader"/> is disposed. </summary>
void OnDispose(IndexReader reader);
}
}
}
7 changes: 5 additions & 2 deletions src/Lucene.Net/Support/ObsoleteAPI/IndexReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public interface IReaderClosedListener
private sealed class ReaderCloseListenerWrapper : IReaderDisposedListener
{
private readonly IReaderClosedListener listener;

public ReaderCloseListenerWrapper(IReaderClosedListener listener)
{
this.listener = listener ?? throw new ArgumentNullException(nameof(listener));
Expand All @@ -47,7 +48,9 @@ public ReaderCloseListenerWrapper(IReaderClosedListener listener)
public void OnDispose(IndexReader reader) => listener.OnClose(reader);

public override bool Equals(object obj) => listener.Equals(obj);

public override int GetHashCode() => listener.GetHashCode();

public override string ToString() => listener.ToString();
}

Expand All @@ -57,7 +60,7 @@ public ReaderCloseListenerWrapper(IReaderClosedListener listener)
/// <para/>
/// @lucene.experimental
/// </summary>
[Obsolete("Use AddReaderDisposedListerner method instead. This method will be removed in 4.8.0 release candidate."), System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)]
[Obsolete("Use AddReaderDisposedListener method instead. This method will be removed in 4.8.0 release candidate."), System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)]
public void AddReaderClosedListener(IReaderClosedListener listener)
{
EnsureOpen();
Expand All @@ -69,7 +72,7 @@ public void AddReaderClosedListener(IReaderClosedListener listener)
/// <para/>
/// @lucene.experimental
/// </summary>
[Obsolete("Use RemoveReaderDisposedListerner method instead. This method will be removed in 4.8.0 release candidate."), System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)]
[Obsolete("Use RemoveReaderDisposedListener method instead. This method will be removed in 4.8.0 release candidate."), System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)]
public void RemoveReaderClosedListener(IReaderClosedListener listener)
{
EnsureOpen();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,13 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>net5.0</TargetFramework>
<TargetFramework>net8.0</TargetFramework>
<RootNamespace>Lucene.Net.CodeAnalysis</RootNamespace>

<IsPublishable>false</IsPublishable>
<IsPublishable Condition=" '$(TargetFramework)' == 'net5.0' ">true</IsPublishable>
<IsPublishable Condition=" '$(TargetFramework)' == 'net8.0' ">true</IsPublishable>
<IsTestProject>true</IsTestProject>

<!-- We purposely test on EoL frameworks for testing netstandard2.1, but we want to keep this warning in production code. -->
<CheckEolTargetFramework>false</CheckEolTargetFramework>
</PropertyGroup>


<Import Project="$(SolutionDir).build/TestReferences.Common.targets" />

Expand Down

0 comments on commit 1854672

Please sign in to comment.