Skip to content
This repository was archived by the owner on Oct 16, 2020. It is now read-only.

Commit c19a83f

Browse files
committed
Fix premature disposal of IProgressMonitor in parallel background search.
This was the cause of "InvalidOperationException: The LinkedList node does not belong to current LinkedList." in ProgressCollector.UnregisterNamedMonitor().
1 parent ef71b3c commit c19a83f

5 files changed

Lines changed: 49 additions & 3 deletions

File tree

src/AddIns/Misc/SearchAndReplace/Project/Engine/SearchManager.cs

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,14 @@ namespace SearchAndReplace
2828
public class SearchManager
2929
{
3030
#region FindAll
31+
/// <summary>
32+
/// Prepares a parallel search operation.
33+
/// </summary>
34+
/// <param name="strategy">The search strategy. Contains the search term and options.</param>
35+
/// <param name="location">The location to search in.</param>
36+
/// <param name="progressMonitor">The progress monitor used to report progress.
37+
/// The parallel search operation will take ownership of this monitor: the monitor is disposed when the search finishes!</param>
38+
/// <returns>Observable that starts performing the search when subscribed to. Does not support more than one subscriber!</returns>
3139
public static IObservable<SearchedFile> FindAllParallel(ISearchStrategy strategy, SearchLocation location, IProgressMonitor progressMonitor)
3240
{
3341
currentSearchRegion = null;
@@ -122,7 +130,7 @@ public IDisposable Subscribe(IObserver<SearchedFile> observer)
122130
observer.OnError(t.Exception);
123131
else
124132
observer.OnCompleted();
125-
this.Dispose();
133+
monitor.Dispose();
126134
});
127135
return this;
128136
}
@@ -181,8 +189,9 @@ void ThrowIfCancellationRequested()
181189

182190
public void Dispose()
183191
{
192+
// Warning: Dispose() may be called concurrently while the operation is still in progress.
193+
// We cannot dispose the progress monitor here because it is still in use by the search operation.
184194
cts.Cancel();
185-
monitor.Dispose();
186195
}
187196

188197
SearchedFile SearchFile(FileName fileName)
@@ -509,6 +518,12 @@ public static bool IsResultSelected(SearchResultMatch result)
509518
&& result.Length == editor.SelectionLength;
510519
}
511520

521+
/// <summary>
522+
/// Shows searchs results in the search results pad, and brings that pad to front.
523+
/// </summary>
524+
/// <param name="pattern">The pattern that is being searched for; used for the title of the search in the list of past searched.</param>
525+
/// <param name="results">An observable that represents a background search operation.
526+
/// The search results pad will subscribe to this observable in order to receive the search results.</param>
512527
public static void ShowSearchResults(string pattern, IObservable<SearchedFile> results)
513528
{
514529
string title = StringParser.Parse("${res:MainWindow.Windows.SearchResultPanel.OccurrencesOf}",

src/Main/Base/Project/Editor/Search/ISearchResultFactory.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,18 @@ namespace ICSharpCode.SharpDevelop.Editor.Search
1212
/// </summary>
1313
public interface ISearchResultFactory
1414
{
15+
/// <summary>
16+
/// Creates an <see cref="ISearchResult"/> object from a list of matches.
17+
/// </summary>
18+
/// <param name="title">The title of the search.</param>
19+
/// <param name="matches">The list of matches. CreateSearchResult() will enumerate once through the IEnumerable in order to retrieve the search results.</param>
1520
ISearchResult CreateSearchResult(string title, IEnumerable<SearchResultMatch> matches);
1621

22+
/// <summary>
23+
/// Creates an <see cref="ISearchResult"/> object for a background search operation.
24+
/// </summary>
25+
/// <param name="title">The title of the search.</param>
26+
/// <param name="matches">The background search operation. CreateSearchResult() will subscribe to the observable in order to retrieve the search results.</param>
1727
ISearchResult CreateSearchResult(string title, IObservable<SearchedFile> matches);
1828
}
1929
}

src/Main/Base/Project/Editor/Search/SearchResultsPad.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,10 @@ public void ClearLastSearchesList()
7878
SD.WinForms.SetContent(contentPlaceholder, null);
7979
}
8080

81+
/// <summary>
82+
/// Shows a search in the search results pad.
83+
/// The previously shown search will be stored in the list of past searches.
84+
/// </summary>
8185
public void ShowSearchResults(ISearchResult result)
8286
{
8387
if (result == null)
@@ -108,18 +112,31 @@ public void ShowSearchResults(ISearchResult result)
108112
SearchResultsShown(this, EventArgs.Empty);
109113
}
110114

115+
/// <summary>
116+
/// Shows a search in the search results pad.
117+
/// The previously shown search will be stored in the list of past searches.
118+
/// </summary>
119+
/// <param name="title">The title of the search.</param>
120+
/// <param name="matches">The list of matches. ShowSearchResults() will enumerate once through the IEnumerable in order to retrieve the search results.</param>
111121
public void ShowSearchResults(string title, IEnumerable<SearchResultMatch> matches)
112122
{
113123
ShowSearchResults(CreateSearchResult(title, matches));
114124
}
115125

126+
/// <summary>
127+
/// Performs a background search in the search results pad.
128+
/// The previously shown search will be stored in the list of past searches.
129+
/// </summary>
130+
/// <param name="title">The title of the search.</param>
131+
/// <param name="matches">The background search operation. ShowSearchResults() will subscribe to the observable in order to retrieve the search results.</param>
116132
public void ShowSearchResults(string title, IObservable<SearchedFile> matches)
117133
{
118134
ShowSearchResults(CreateSearchResult(title, matches));
119135
}
120136

121137
public event EventHandler SearchResultsShown = delegate {};
122138

139+
/// <inheritdoc cref="ISearchResultFactory.CreateSearchResult(string,IEnumerable{SearchResultMatch})"/>
123140
public static ISearchResult CreateSearchResult(string title, IEnumerable<SearchResultMatch> matches)
124141
{
125142
if (title == null)
@@ -135,6 +152,7 @@ public static ISearchResult CreateSearchResult(string title, IEnumerable<SearchR
135152
}
136153

137154

155+
/// <inheritdoc cref="ISearchResultFactory.CreateSearchResult(string,IObservable{SearchResultMatch})"/>
138156
public static ISearchResult CreateSearchResult(string title, IObservable<SearchedFile> matches)
139157
{
140158
if (title == null)

src/Main/Base/Project/Util/IProgressMonitor.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public interface IProgressMonitor : IDisposable, IProgress<double>
3939
/// </summary>
4040
/// <param name="workAmount">The amount of work this sub-task performs in relation to the work of this task.
4141
/// That means, this parameter is used as a scaling factor for work performed within the subtask.</param>
42-
/// <param name="childCancellationToken">
42+
/// <param name="cancellationToken">
4343
/// A cancellation token that can be used to cancel the sub-task.
4444
/// Note: cancelling the main task will not cancel the sub-task.
4545
/// </param>

src/Main/Base/Project/Util/ProgressCollector.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,9 @@ void UnregisterNamedMonitor(LinkedListNode<string> nameEntry)
191191
{
192192
lock (namedMonitors) {
193193
bool wasFirst = namedMonitors.First == nameEntry;
194+
// Note: if Remove() crashes with "InvalidOperationException: The LinkedList node does not belong to current LinkedList.",
195+
// that's an indication that the progress monitor is being disposed multiple times concurrently.
196+
// (which is not allowed according to IProgressMonitor thread-safety documentation)
194197
namedMonitors.Remove(nameEntry);
195198
if (wasFirst)
196199
SetTaskName(namedMonitors.First != null ? namedMonitors.First.Value : null);

0 commit comments

Comments
 (0)