Fix environment exit & Wait image cache save before restarting & Add settings saving lock & Do not crash when caught exception saving

[Jack251970]

Fix Environment.Exit(0) issue

Since #3367 has introduced Application.Current.Shutdown for app dispose, using Environment.Exit(0) will cause issue.

// If we call Environment.Exit(0), the application dispose will be called before _mainWindow.Close()
// Accessing _mainWindow?.Dispatcher will cause the application stuck
// So here we need to check it and just return so that we will not acees _mainWindow?.Dispatcher

Wait image cache save

Before restarting app, we should wait ImageLoader.SaveAsync.

Add settings save lock

Make multiple thread safe.

Do not crash when caught exception in saving

Use try when calling JsonStorage and BinaryStorage save.

Test

• Install original dev version and install one plugin. When application is exited for restarting, it will stuck. Using installer in this PR, the issue is fixed.

[Jack251970]

Sorry, I forget to fix typos here and please review.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (3)

Flow.Launcher.Infrastructure/Storage/PluginJsonStorage.cs (1)

14-19: Consider using Lazy<T> for thread-safe API initialization.

The current lazy initialization approach works, but isn't thread-safe. If multiple threads access the API property simultaneously, you could end up with multiple instances.

-private static IPublicAPI api = null;
-private static IPublicAPI API => api ??= Ioc.Default.GetRequiredService<IPublicAPI>();
+private static readonly Lazy<IPublicAPI> lazyApi = new Lazy<IPublicAPI>(() => Ioc.Default.GetRequiredService<IPublicAPI>());
+private static IPublicAPI API => lazyApi.Value;

Flow.Launcher.Infrastructure/Storage/FlowLauncherJsonStorage.cs (2)

11-16: Consider using Lazy<T> for thread-safe API initialization.

The current lazy initialization approach works, but isn't thread-safe. If multiple threads access the API property simultaneously, you could end up with multiple instances.

-private static IPublicAPI api = null;
-private static IPublicAPI API => api ??= Ioc.Default.GetRequiredService<IPublicAPI>();
+private static readonly Lazy<IPublicAPI> lazyApi = new Lazy<IPublicAPI>(() => Ioc.Default.GetRequiredService<IPublicAPI>());
+private static IPublicAPI API => lazyApi.Value;

Don't forget to add using System; at the top of the file to support Lazy<T>.

---

1-51: Consider reducing code duplication between storage classes.

The error handling implementation in both FlowLauncherJsonStorage and PluginJsonStorage is nearly identical. Consider refactoring to reduce duplication.

You could:

1. Create a common base class with error handling
2. Use a composition approach with an error handler service
3. Create extension methods for adding error handling to Task objects

If base class methods are non-virtual, the current approach with new methods is appropriate.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ec7a4e8 and 9b0d3f2.

📒 Files selected for processing (3)

• Flow.Launcher.Core/Updater.cs (4 hunks)
• Flow.Launcher.Infrastructure/Storage/FlowLauncherJsonStorage.cs (2 hunks)
• Flow.Launcher.Infrastructure/Storage/PluginJsonStorage.cs (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• Flow.Launcher.Core/Updater.cs

🧰 Additional context used

🧬 Code Definitions (2)

Flow.Launcher.Infrastructure/Storage/PluginJsonStorage.cs (2)

Flow.Launcher.Infrastructure/Storage/FlowLauncherJsonStorage.cs (2)

• Save (26-36)
• Task (38-48)

Flow.Launcher.Infrastructure/Storage/JsonStorage.cs (3)

• Save (182-190)
• Task (44-73)
• Task (192-198)

Flow.Launcher.Infrastructure/Storage/FlowLauncherJsonStorage.cs (3)

Flow.Launcher.Infrastructure/Storage/JsonStorage.cs (9)

• T (113-139)
• T (141-149)
• T (151-169)
• JsonStorage (15-212)
• JsonStorage (32-34)
• JsonStorage (36-42)
• Save (182-190)
• Task (44-73)
• Task (192-198)

Flow.Launcher.Infrastructure/Storage/PluginJsonStorage.cs (2)

• Save (36-46)
• Task (48-58)

Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs (3)

• Save (137-147)
• Task (38-85)
• Task (125-135)

⏰ Context from checks skipped due to timeout of 90000ms (1)

• GitHub Check: gitStream.cm

🔇 Additional comments (6)

Flow.Launcher.Infrastructure/Storage/PluginJsonStorage.cs (3)

2-5: Added necessary import statements.

The new imports are correctly added to support the new functionality for asynchronous operations, dependency injection, and plugin API access.

---

48-58: Improved error handling for the SaveAsync method.

The asynchronous version correctly implements exception handling similar to the synchronous version, maintaining consistency in error handling approach.

---

36-46:

✅ Verification successful

Improved error handling for the Save method.

Good addition of try-catch block to prevent crashes when saving settings fails. The error is properly logged instead of being silently swallowed.

Verify this implementation aligns with the PR objectives of preventing crashes during saving operations.

---

🏁 Script executed:

#!/bin/bash
# Check if the base class methods are virtual or override-able
rg -A 1 -B 1 "public\s+(virtual\s+)?(void\s+Save|Task\s+SaveAsync)" Flow.Launcher.Infrastructure/Storage/JsonStorage.cs

Length of output: 159

---

Verification Complete: Enhanced Error Handling Implementation

The implementation of the Save method in PluginJsonStorage.cs now includes robust error handling. The try-catch block prevents crashes by catching exceptions from the non-virtual base.Save() method (as confirmed in Flow.Launcher.Infrastructure/Storage/JsonStorage.cs). The error is appropriately logged with the class name and file path, aligning with the PR objectives.

• No further changes are needed as the use of the new keyword is valid given that the base method is not virtual.

Flow.Launcher.Infrastructure/Storage/FlowLauncherJsonStorage.cs (3)

2-5: Added necessary import statements.

The new imports are correctly added to support the new functionality for asynchronous operations, dependency injection, and plugin API access.

---

26-36: Improved error handling for the Save method.

Good addition of try-catch block to prevent crashes when saving settings fails. The error is properly logged instead of being silently swallowed.

---

38-48: Improved error handling for the SaveAsync method.

The asynchronous version correctly implements exception handling similar to the synchronous version, maintaining consistency in error handling approach.

[github-actions]

@check-spelling-bot Report

🔴 Please review

See the 📂 files view, the 📜action log, or 📝 job summary for details.

❌ Errors	Count
❌ forbidden-pattern	24
⚠️ non-alpha-in-dictionary	19

See ❌ Event descriptions for more information.

Forbidden patterns 🙅 (1)

In order to address this, you could change the content to not match the forbidden patterns (comments before forbidden patterns may help explain why they're forbidden), add patterns for acceptable instances, or adjust the forbidden patterns themselves.

These forbidden patterns matched content:

s.b. workaround(s)

\bwork[- ]arounds?\b

If the flagged items are 🤯 false positives

If items relate to a ...

• binary file (or some other file you wouldn't want to check at all).

  Please add a file path to the excludes.txt file matching the containing file.

  File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

  ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

• well-formed pattern.

  If you can write a pattern that would match it,
  try adding it to the patterns.txt file.

  Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

  Note that patterns can't match multiline strings.

[Jack251970]

Sorry for asking your review again. I didn't handle conflict resolution well causing me to need an additional commit to fix it.