Fix possible null reference when query is cancelled & Add new image icon for images whose formats are not support & Other improvements #3099
Conversation
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughThe pull request introduces several enhancements across multiple classes. The Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (5)
Flow.Launcher/Storage/TopMostRecord.cs (3)
Line range hint
1-8: Critical: Address thread safety concernsThe TODO comment indicates that this class is accessed from multiple threads but lacks thread safety mechanisms. This could lead to race conditions and data corruption when concurrent operations modify the
recordsdictionary.Consider these thread-safety options:
- Use
ConcurrentDictionary<string, Record>instead ofDictionary<string, Record>- Add lock mechanisms around dictionary operations
- Implement the double-checked locking pattern for read operations
Example implementation using
ConcurrentDictionary:- public Dictionary<string, Record> records { get; private set; } = new Dictionary<string, Record>(); + public ConcurrentDictionary<string, Record> records { get; private set; } = new ConcurrentDictionary<string, Record>();
Line range hint
15-21: Enhance null-safety in IsTopMost methodWhile the added null check for
result.OriginQueryis good, the method still has potential null reference issues:
- No null check for the
resultparameter itself- The second return statement could throw if
result.OriginQuerybecomes null between checksConsider this safer implementation:
internal bool IsTopMost(Result result) { - if (records.Count == 0 || (result.OriginQuery != null && !records.ContainsKey(result.OriginQuery.RawQuery))) + if (result?.OriginQuery == null || records.Count == 0 || !records.ContainsKey(result.OriginQuery.RawQuery)) { return false; } // since this dictionary should be very small (or empty) going over it should be pretty fast. return records[result.OriginQuery.RawQuery].Equals(result); }
Line range hint
23-39: Add null checks to Remove and AddOrUpdate methodsBoth methods directly access
result.OriginQuery.RawQuerywithout null checks, which could lead to null reference exceptions when handling cancelled queries.Suggested implementation:
internal void Remove(Result result) { + if (result?.OriginQuery == null) return; records.Remove(result.OriginQuery.RawQuery); } internal void AddOrUpdate(Result result) { + if (result?.OriginQuery == null) return; var record = new Record { PluginID = result.PluginID, Title = result.Title, SubTitle = result.SubTitle }; records[result.OriginQuery.RawQuery] = record; }Flow.Launcher.Infrastructure/Image/ImageLoader.cs (2)
218-227: Good addition of specific exception handling for unsupported formats.The try-catch block appropriately handles NotSupportedException for unsupported image formats, preventing crashes and improving robustness.
However, consider these improvements:
- Instead of setting
image = null, use the MissingImage as fallback- Consider catching other potential exceptions (e.g., OutOfMemoryException) that could occur during full image loading
try { image = LoadFullImage(path); type = ImageType.FullImageFile; } -catch (NotSupportedException) +catch (NotSupportedException ex) { - image = null; + Log.Debug($"|ImageLoader.GetThumbnailResult|Unsupported image format for {path}: {ex.Message}"); + image = ImageCache[Constant.MissingImgIcon, false]; type = ImageType.Error; } +catch (Exception ex) when (ex is OutOfMemoryException or InvalidOperationException) +{ + Log.Exception($"|ImageLoader.GetThumbnailResult|Failed to load full image {path}", ex); + image = ImageCache[Constant.MissingImgIcon, false]; + type = ImageType.Error; +}
220-220: Consider optimizing image format validation.The
LoadFullImagemethod makes multiple attempts to load and resize the image. For unsupported formats, this could mean multiple unnecessary attempts before failing.Consider validating the image format before attempting to load:
+private static bool IsFormatSupported(string path) +{ + try + { + using var stream = File.OpenRead(path); + using var decoder = BitmapDecoder.Create( + stream, + BitmapCreateOptions.IgnoreColorProfile, + BitmapCacheOption.None); + return true; + } + catch (NotSupportedException) + { + return false; + } +} try { + if (!IsFormatSupported(path)) + { + image = ImageCache[Constant.MissingImgIcon, false]; + type = ImageType.Error; + return; + } image = LoadFullImage(path); type = ImageType.FullImageFile; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
Flow.Launcher.Infrastructure/Image/ImageLoader.cs(1 hunks)Flow.Launcher/Storage/TopMostRecord.cs(1 hunks)
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
🥷 Code experts: onesounds onesounds has most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
Flow.Launcher/Storage/TopMostRecord.cs (2)
Line range hint
30-37: Add null check and refactor record creationTwo issues to address:
- Missing null check for
result.OriginQuery- Record creation logic could be extracted for reusability
Apply these changes:
+ private static Record CreateRecord(Result result) => new Record + { + PluginID = result.PluginID, + Title = result.Title, + SubTitle = result.SubTitle + }; internal void AddOrUpdate(Result result) { - var record = new Record - { - PluginID = result.PluginID, - Title = result.Title, - SubTitle = result.SubTitle - }; - records.AddOrUpdate(result.OriginQuery.RawQuery, record, (key, oldValue) => record); + if (result.OriginQuery?.RawQuery == null) return; + var record = CreateRecord(result); + records.AddOrUpdate(result.OriginQuery.RawQuery, record, (_, _) => record); }
42-42: Add parameter validation and optimize initializationConsider adding parameter validation and capacity planning for better performance.
Apply this change:
public void Load(Dictionary<string, Record> dictionary) { - records = new ConcurrentDictionary<string, Record>(dictionary); + if (dictionary == null) + throw new ArgumentNullException(nameof(dictionary)); + + var concurrencyLevel = Environment.ProcessorCount; + var capacity = dictionary.Count; + records = new ConcurrentDictionary<string, Record>(concurrencyLevel, capacity); + foreach (var kvp in dictionary) + { + records.TryAdd(kvp.Key, kvp.Value); + } }Flow.Launcher/Storage/UserSelectedRecord.cs (1)
Line range hint
1-109: Consider using ConcurrentDictionary for thread safety.The class uses regular
Dictionaryfor storing records which isn't thread-safe. Since the PR introduces thread safety improvements elsewhere usingConcurrentDictionary, consider applying the same pattern here to prevent potential race conditions inAdd()andGetSelectedCount()methods.Here's the suggested implementation:
- public Dictionary<int, int> recordsWithQuery { get; private set; } + public ConcurrentDictionary<int, int> recordsWithQuery { get; private set; } public UserSelectedRecord() { - recordsWithQuery = new Dictionary<int, int>(); + recordsWithQuery = new ConcurrentDictionary<int, int>(); } public void Add(Result result) { TransformOldRecords(); var keyWithQuery = GenerateQueryAndResultHashCode(result.OriginQuery, result); - if (!recordsWithQuery.TryAdd(keyWithQuery, 1)) - recordsWithQuery[keyWithQuery]++; + recordsWithQuery.AddOrUpdate(keyWithQuery, 1, (_, count) => count + 1); var keyWithoutQuery = GenerateResultHashCode(result); - if (!recordsWithQuery.TryAdd(keyWithoutQuery, 1)) - recordsWithQuery[keyWithoutQuery]++; + recordsWithQuery.AddOrUpdate(keyWithoutQuery, 1, (_, count) => count + 1); }Flow.Launcher/ViewModel/MainViewModel.cs (2)
1450-1476: Consider enhancing error handling and logging.While the addition of error handling is good, there are a few improvements that could be made:
- Consider catching specific exceptions rather than using a general catch-all
- The debug log could be enhanced to include the full exception details including stack trace for better troubleshooting
Here's a suggested improvement:
try { foreach (var metaResults in resultsForUpdates) { foreach (var result in metaResults.Results) { if (_topMostRecord.IsTopMost(result)) { result.Score = int.MaxValue; } else { var priorityScore = metaResults.Metadata.Priority * 150; result.Score += _userSelectedRecord.GetSelectedCount(result) + priorityScore; } } } bool reSelect = resultsForUpdates.First().ReSelectFirstResult; Results.AddResults(resultsForUpdates, token, reSelect); } -catch (Exception ex) +catch (InvalidOperationException ex) { - Log.Debug("MainViewModel", $"Error in UpdateResultView: {ex.Message}"); + Log.Error("MainViewModel", "Error in UpdateResultView", ex); +} +catch (Exception ex) +{ + Log.Error("MainViewModel", "Unexpected error in UpdateResultView", ex); }
1456-1464: Consider extracting magic numbers into constants.The scoring logic uses a magic number (150) for priority score calculation. This should be extracted into a named constant for better maintainability and clarity.
+private const int PRIORITY_SCORE_MULTIPLIER = 150; + if (_topMostRecord.IsTopMost(result)) { result.Score = int.MaxValue; } else { - var priorityScore = metaResults.Metadata.Priority * 150; + var priorityScore = metaResults.Metadata.Priority * PRIORITY_SCORE_MULTIPLIER; result.Score += _userSelectedRecord.GetSelectedCount(result) + priorityScore; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
Flow.Launcher/MainWindow.xaml(1 hunks)Flow.Launcher/Storage/TopMostRecord.cs(2 hunks)Flow.Launcher/Storage/UserSelectedRecord.cs(2 hunks)Flow.Launcher/ViewModel/MainViewModel.cs(6 hunks)
🔇 Additional comments (6)
Flow.Launcher/Storage/TopMostRecord.cs (3)
1-1: LGTM: Good improvement for thread safety
The switch to ConcurrentDictionary is a solid improvement that will prevent potential race conditions in a multi-threaded environment.
Also applies to: 11-11
15-21:
Strengthen null check protection
While the initial null check is good, the return statement still directly accesses result.OriginQuery.RawQuery without a null check, which could throw a NullReferenceException.
Apply this fix:
- return result.OriginQuery != null && records[result.OriginQuery.RawQuery].Equals(result);
+ if (result.OriginQuery?.RawQuery == null) return false;
+ return records.TryGetValue(result.OriginQuery.RawQuery, out var record) && record.Equals(result);9-11: Verify concurrent access patterns
Let's verify how this class is accessed concurrently to ensure our thread safety improvements are sufficient.
✅ Verification successful
Let me gather more information about how TopMostRecord is used in MainViewModel.cs to understand the concurrent access patterns better.
Based on the analysis of the code, I can now generate the final response:
Thread safety implementation is appropriate
The ConcurrentDictionary usage in TopMostRecord is well-justified. The evidence shows that:
- TopMostRecord is accessed from multiple contexts including async query operations and UI operations
- The access patterns are simple CRUD operations (AddOrUpdate, Remove, IsTopMost) which are all thread-safe with ConcurrentDictionary
- The class is used in result scoring during async query operations while also being modified through UI actions
- All operations on _topMostRecord are atomic and don't require additional synchronization
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usages of TopMostRecord class
rg "TopMostRecord" -B 2 -A 2
# Search for async/await patterns near TopMostRecord usage
ast-grep --pattern 'async $_ $_($$$) {
$$$
TopMostRecord
$$$
}'
Length of output: 1863
Script:
#!/bin/bash
# Look for methods in MainViewModel that use _topMostRecord
rg "_topMostRecord\." -B 5 -A 5 "Flow.Launcher/ViewModel/MainViewModel.cs"
# Search for any async methods in MainViewModel
ast-grep --pattern 'async $_ $_($$$) {
$$$
}' "Flow.Launcher/ViewModel/MainViewModel.cs"
# Look for any thread-related code near TopMostRecord usage
rg "Task|Thread|Parallel|async|await" -B 2 -A 2 "Flow.Launcher/ViewModel/MainViewModel.cs"
Length of output: 11432
Flow.Launcher/Storage/UserSelectedRecord.cs (1)
54-58: LGTM! Null check prevents NullReferenceException.
The added null check properly handles cancelled queries by falling back to result-only hash code generation, preventing the NullReferenceException when accessing query properties.
Flow.Launcher/MainWindow.xaml (1)
311-311: LGTM! Verify ViewModel property binding.
The change from PluginIconPath to PluginIconSource aligns with the PR objectives to improve image format support and null reference handling.
Let's verify the corresponding ViewModel property exists:
✅ Verification successful
The previous command might have failed due to the pattern syntax. Let's try a broader search to:
- Find any occurrence of PluginIconSource property
- Locate the MainViewModel file to verify the property exists
Property binding verified and implemented correctly ✓
The PluginIconSource property exists in MainViewModel.cs with proper implementation:
- Defined as
public ImageSource PluginIconSource { get; private set; } - Properly initialized and managed alongside
PluginIconPath - Used with
ImageLoader.LoadAsync()for loading the image source
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the PluginIconSource property exists in MainViewModel
ast-grep --pattern 'class MainViewModel {
$$$
public ImageSource PluginIconSource { get; set; }
$$$
}'
Length of output: 110
Script:
#!/bin/bash
# Search for PluginIconSource property in all files
rg "PluginIconSource" -A 2 -B 2
# Search for files containing MainViewModel to ensure we're looking in the right places
fd MainViewModel
Length of output: 2148
Flow.Launcher/ViewModel/MainViewModel.cs (1)
26-27: LGTM: Image handling changes look good!
The addition of PluginIconSource property and related imports properly supports asynchronous image loading while maintaining null safety.
Also applies to: 727-728
Jack251970
changed the title
|
@Jack251970 I don't want to swipe your interest! It is always a tough time (and discouraging) waiting people to review your prs. Just to let you know I will be a bit busy before 12/3. Afterward I will try my best to review your pr asap. If I forget afterward, ping me at discord😁. (Or maybe other people in our teams will review your pr earlier). |
|
@taooceros No worries at all! I completely understand that reviewing PRs can take time, especially with a busy schedule. |
|
@onesounds how do you think about the not supported image? |
| #endif | ||
|
|
||
| foreach (var metaResults in resultsForUpdates) | ||
| try |
There was a problem hiding this comment.
could you explain for the try?
There was a problem hiding this comment.
It could happen when calling ResultsUpdated.Invoke event in IResultUpdate interface so fast that main view model cannot complete update event (still execute foreach sentence).
Take WebSearch plugin for example,
public async Task<List<Result>> QueryAsync(Query query, CancellationToken token)
{
var results = new List<Result>();
foreach (...)
{
...
results.Add(result); // edit this List
...
// Here you use this List to give to MainViewModel for updating
// If you do the time-consuming work after this so fast, you will edit this list when updating the MainViewModel
// So we need to use Try to avoid this problem
ResultsUpdated?.Invoke(this, new ResultUpdatedEventArgs
{
Results = results,
Query = query
});
... // do time-consuming work
}
return results;
}
There was a problem hiding this comment.
Make sense, but that also means we need additional synchronization? I remember the ConcurrentDictionary is already providing a enumerator that is thread safe?
There was a problem hiding this comment.
The key of the problem is that this plugin has not tell the developer to do thread-safe operation in QueryAsync (like return a copy of the list as the Results in ResultUpdatedEventArgs). So the developer will possibly return the same List to the ResultUpdatedEventArgs and the result of the QueryAsync function.
If you want to fully address this problem, you need to add additional synchronization.
For example, you can change the definition of the ResultUpdatedEventArgs to
public class ResultUpdatedEventArgs : EventArgs
{
public ConcurrentBag<Result> Results; // thread-safe
public Query Query;
public CancellationToken Token { get; init; }
}
Or, you need to ask the developer to return a copy of the List so that the plugin will not change the List for updating the view.
Take WebSearch plugin for example,
public async Task<List<Result>> QueryAsync(Query query, CancellationToken token)
{
var results = new List<Result>();
foreach (...)
{
...
results.Add(result);
...
var resultsCopy = results.Clone(); // create a copy of the results
ResultsUpdated?.Invoke(this, new ResultUpdatedEventArgs
{
Results = resultsCopy,
Query = query
});
...
}
return results;
}
Anyway, I think this is the most developer-friendly solution for this problem and developers don not need to redevelop their plugins to fix this.
There was a problem hiding this comment.
@taooceros What do you mean by ConcurrentDictionary and ConcurrentBag?
There was a problem hiding this comment.
Hold on, I remember this is a producer/consumer model with appropriate synchronization through the Channel class. Let me recheck.
There was a problem hiding this comment.
I am a little bit suspicious of this change. Is it possible for you to revert just this part so we can merge first. Then Include it in a new pr so we can discuss specifically about the potential synchronization problem.
There was a problem hiding this comment.
@taooceros Sure! Please check!
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
Flow.Launcher/Storage/TopMostRecord.cs (2)
43-43: Add parameter validationConsider adding parameter validation to handle null dictionary input.
Apply this change:
public void Load(Dictionary<string, Record> dictionary) { + if (dictionary == null) + { + records = new ConcurrentDictionary<string, Record>(); + return; + } records = new ConcurrentDictionary<string, Record>(dictionary); }
Line range hint
9-44: Consider architectural improvements
- Extract common null checking logic into a helper method to reduce code duplication:
private string GetValidQueryKey(Result result) { return result?.OriginQuery?.RawQuery; }
- Consider adding logging for debugging purposes, especially around null queries and concurrent operations.
This will improve maintainability and make debugging easier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
Flow.Launcher/Storage/TopMostRecord.cs(2 hunks)
🔇 Additional comments (3)
Flow.Launcher/Storage/TopMostRecord.cs (3)
1-1: LGTM! Good improvement for thread safety
The switch to ConcurrentDictionary is a solid improvement that will help prevent race conditions when multiple threads access the records collection.
Also applies to: 11-11
15-22: LGTM! Robust null checking and thread-safe access
The changes properly handle:
- Empty collection check
- Null query check
- Thread-safe value retrieval using TryGetValue
27-27:
Add null check to prevent NullReferenceException
The method is still vulnerable to NullReferenceException when result.OriginQuery is null.
Apply this fix:
- records.Remove(result.OriginQuery.RawQuery, out _);
+ if (result?.OriginQuery?.RawQuery != null)
+ {
+ records.TryRemove(result.OriginQuery.RawQuery, out _);
+ }
Blank area is not good. It's weird that there's no image at all. If there's an imgmissing icon like an appmissing icon, ok. |
|
@onesounds I will try to provide an image to show this message. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
Flow.Launcher.Infrastructure/Constant.cs (1)
34-34: Consider adding XML documentation.To maintain consistency and clarity, consider adding XML documentation to explain when this icon is used (i.e., for unsupported image formats).
+ /// <summary> + /// Icon displayed when an image format is not supported for preview. + /// </summary> public static readonly string NoImageIcon = Path.Combine(ImagesDirectory, "no_image.png");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
Flow.Launcher/Images/no_image.pngis excluded by!**/*.png
📒 Files selected for processing (2)
Flow.Launcher.Infrastructure/Constant.cs(1 hunks)Flow.Launcher.Infrastructure/Image/ImageLoader.cs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Flow.Launcher.Infrastructure/Image/ImageLoader.cs
🔇 Additional comments (1)
Flow.Launcher.Infrastructure/Constant.cs (1)
34-34: LGTM! Verify the image file exists.
The new constant follows the established pattern and is correctly implemented using Path.Combine.
Let's verify the image file exists:
✅ Verification successful
The no_image.png file exists and is correctly placed
The file no_image.png (2051 bytes) exists in the main Flow.Launcher/Images/ directory, which is the correct location referenced by the ImagesDirectory constant. The implementation is verified to be working as intended.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the no_image.png file exists in the Images directory
# Find the Images directory and check for no_image.png
fd -t f "no_image.png" -p "Images/"
Length of output: 128
Script:
#!/bin/bash
# Let's first find the actual Images directory location
fd -t d "Images" --full-path
# Then let's check the contents of any Images directories
fd -t d "Images" --exec ls -la {}
Length of output: 10021
|
@taooceros Have got a new icon for the images whose formats are not supported. |
|
@onesounds What do you think of this icon? 
|
|
It's not bad, but I don't think it's the best. Isn't there this icon? |
|
@onesounds Please take a look in https://icons8.com/icon/set/image/fluency. Any image recommended? |
|
How about this? Images exist and can be opened, only previews are not possible. The red line feels like there's something wrong with the file. ((I look good on the right) |
|
@onesounds Yeah, the right icon is better than my original icon with red line. And for this icon called Broken Image: I search on web and I have learnt that unsupported file format is one kind of the causes of the broken images. Perhaps this can let user know this image is broken and cannot be previewed for some reasons? |
|
@onesounds And thanks for your help! |
If the file is not normal, you can use the icon you chose. But if it's a file that the preview doesn't support, it should just be an image icon. (Windows also process previews as icons if possible, and other images as image icons.) I understand that there is no indication of broken files. I won't distinguish it because it's based on extensions. You don't show a file in the preview called a "broken file." You shouldn't choose an icon that can be misunderstood like this. "The issues of 'a corrupted file' and 'unable to display a preview' are separate. Therefore, using an icon for 'file corruption' to represent 'unable to display a preview' is inappropriate. (I am traveling, so I haven’t tested this PR overseas.) If this PR only applies to completely broken images, you may use the icon you selected. |
This comment has been minimized.
This comment has been minimized.
|
Now updated the image and will show the preview like this: |
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
Flow.Launcher/ViewModel/MainViewModel.cs(5 hunks)
🔇 Additional comments (2)
Flow.Launcher/ViewModel/MainViewModel.cs (2)
727-728: LGTM: Well-designed property for plugin icon source
The property is correctly implemented as read-only with private setter and appropriate default value.
1073-1073: Verify the status of concurrent access fixes
Based on previous review comments, there were discussions about using ConcurrentDictionary/ConcurrentBag for thread-safe operations. Let's verify if these changes were implemented elsewhere.
✅ Verification successful
Let me gather more information about the concurrent access implementation in the MainViewModel.cs file to verify if the changes were properly implemented.
Based on the search results, I can now provide a final response:
Concurrent access has been properly implemented
The codebase shows a well-structured approach to handle concurrent access:
- Uses a Channel-based architecture for result updates via
ResultsForUpdateand_resultsUpdateChannelWriter - Implements thread-safe collections where needed (e.g.,
TopMostRecordusesConcurrentDictionary) - Results updates are processed through a dedicated task (
_resultsViewUpdateTask) that safely handles collection modifications - Collection modifications (Clear/Add) are performed through the channel system, preventing direct concurrent access
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for concurrent collection usage in the codebase
# Search for ConcurrentDictionary usage
echo "Searching for ConcurrentDictionary usage..."
rg "ConcurrentDictionary" -A 5
# Search for ConcurrentBag usage
echo "Searching for ConcurrentBag usage..."
rg "ConcurrentBag" -A 5
# Search for synchronization related changes in TopMostRecord
echo "Searching for TopMostRecord implementation..."
rg "class TopMostRecord" -A 10
Length of output: 6617
Script:
#!/bin/bash
# Search for concurrent access related code in MainViewModel.cs
echo "Searching for result collection handling in MainViewModel.cs..."
rg "Results" -A 5 "Flow.Launcher/ViewModel/MainViewModel.cs"
# Search for any lock or synchronization mechanisms
echo "Searching for synchronization mechanisms..."
rg "lock\s*\(" -A 5 "Flow.Launcher/ViewModel/MainViewModel.cs"
# Search for collection modifications
echo "Searching for collection modifications..."
rg "Clear|Add|Remove" -A 5 "Flow.Launcher/ViewModel/MainViewModel.cs"
Length of output: 18643
|
@Jack251970 Could you move the reverted change to a new pr? |
|
@taooceros Moved. Please check #3115. |
4 participants
This PR contains those fixes and improvements
It will happen when one query is cancelled and can cause query bar not working like this:
https://github.com/user-attachments/assets/8a0a2c0f-6db6-4061-8ff6-a979408fc6ff
Throw exception from VS:
It add support for images whose formats are not support for preview like this:
Before it is catch in outer try catch sentences, and the icon of images will be
AppMissingIcon.png, which is quite strange for.pngextension.Now show the preview like this:
Introduce concurrent directionary for topmost record.
Make sure TopMostRecord is thread safe.
Fix modified collection enumeration issue when updating results, which can also cause the same problem like 1.
It could happen when calling
ResultsUpdated.Invokeevent inIResultUpdateinterface so fast that main view model cannot complete update event (still executeforeachsentence).Fix xaml convert issue and use icon loader for PluginActivationIcon.