Framework Fixes by MidoriKami · Pull Request #2910 · goatcorp/Dalamud

MidoriKami · 2026-08-07T23:24:05Z

The primary goal of this PR is to reduce/eliminate various async footguns when using the Framework service.

The Problem

It is very unintuitive that await IFramework.RunOnFrameworkThread(...) would cause all code executed after the call to be on the main thread. This has shown to be a mistake that multiple plugin devs have made and will likely continue to make as IAsyncDalamudPlugin becomes more widely used.

IFramework.Run(...)

Changes the functionality of IFramework.Run(...) to execute immediately if already on the main thread, this prevents the deadlock that can occur if you accidentally await IFramework.Run(...) while already on the main thread, as it will run the delegate immediately, and then return Task.CompletedTask.

IFramework.RunOnTick(...)

Additionally changes the functionality of IFramework.RunOnTick(...) to both use async functionality and prevents awaiting the call from returning you back to the main thread as was the problem with await IFramework.RunOnFrameworkThread(...).

The result of these changes is IFramework Run/RunOnTick will behave intuitively, and safely.

IFramework.RunOnFrameworkThread

Additionally I have set obsoletes that have been around since 2024, to Obsolete with error, and marked IFramework.RunOnFrameworkThread as obsolete (without error).

The actual behavior of IFramework.RunOnFrameworkThread is unchanged, it will continue to cause issues and confusion from people that are using this call until they switch to using Run or RunOnTick.

This is intended to be implemented as a non-breaking change, but as a improved behavior change. I hope this can be merged sooner than later, as some of these fixes will improve the overall experience for users and devs.

Cancellation Tokens

The way cancellation tokens are handled is a little different now, and performs much more intuitively. Linked CancellationTokens are now used allowing cancellation from both the provided token and FrameworkDestroyed cancellation token at the same time.

Additionally, FrameworkPluginScoped now has a PluginUnloadingCancellation token added, that is cancelled when the scoped service is being unloaded, resulting in logical task cancellation when the plugin is being unloaded in addition to any provided cancellation token and the FrameworkDestroyed cancellation token.

Testing

This was tested using VanillaPlus as a testing platform running async from the game during initial load with the following code:

Testing using RunOnTick

    public override async Task OnEnableAsync() {
        IPluginLog.Get().Debug($"Root - IsMainThread: {ThreadSafety.IsMainThread}");

        await IFramework.Get().RunOnTick(async () => {
            IPluginLog.Get().Debug($"DeadlockTest - IsMainThread: {ThreadSafety.IsMainThread}");

            await IFramework.Get().RunOnTick(() => {
                IPluginLog.Get().Debug("DID WE DEADLOCK?");
                IPluginLog.Get().Debug($"DeadlockTest INNER - IsMainThread: {ThreadSafety.IsMainThread}");
            }, delayTicks: 2);

            IPluginLog.Get().Debug($"DeadlockTest Post-Nesting - IsMainThread: {ThreadSafety.IsMainThread}");
        }, delayTicks: 2);

        IPluginLog.Get().Debug($"Root - IsMainThread: {ThreadSafety.IsMainThread}");

        await IFramework.Get().RunOnTick(() => {
            IPluginLog.Get().Debug($"Action - IsMainThread: {ThreadSafety.IsMainThread}");
        }, delayTicks: 2);

        IPluginLog.Get().Debug($"Root - IsMainThread: {ThreadSafety.IsMainThread}");

        await IFramework.Get().RunOnTick(() => {
            IPluginLog.Get().Debug($"Func - IsMainThread: {ThreadSafety.IsMainThread}");
            return true;
        }, delay: TimeSpan.FromMilliseconds(500));

        IPluginLog.Get().Debug($"Root - IsMainThread: {ThreadSafety.IsMainThread}");

        await IFramework.Get().RunOnTick(async () => {
            IPluginLog.Get().Debug($"Task<T> - IsMainThread: {ThreadSafety.IsMainThread}");
            return true;
        }, delay: TimeSpan.FromMilliseconds(500));

        IPluginLog.Get().Debug($"Root - IsMainThread: {ThreadSafety.IsMainThread}");

        await IFramework.Get().RunOnTick(async () => {
            IPluginLog.Get().Debug($"Task - IsMainThread: {ThreadSafety.IsMainThread}");
        }, delay: TimeSpan.FromMilliseconds(500));

        IPluginLog.Get().Debug($"Root - IsMainThread: {ThreadSafety.IsMainThread}");
    }

And the following log output:

16:22:49.944 | INF | [VanillaPlus] Enabling DebugGameModification
16:22:49.944 | DBG | [VanillaPlus] Root - IsMainThread: False
16:22:49.961 | DBG | [VanillaPlus] DeadlockTest - IsMainThread: True
16:22:49.961 | DBG | [VanillaPlus] Root - IsMainThread: False
16:22:49.986 | DBG | [VanillaPlus] DID WE DEADLOCK?
16:22:49.986 | DBG | [VanillaPlus] DeadlockTest INNER - IsMainThread: True
16:22:49.986 | DBG | [VanillaPlus] DeadlockTest Post-Nesting - IsMainThread: False
16:22:49.986 | DBG | [VanillaPlus] Action - IsMainThread: True
16:22:49.986 | DBG | [VanillaPlus] Root - IsMainThread: False
16:22:50.491 | DBG | [VanillaPlus] Func - IsMainThread: True
16:22:50.491 | DBG | [VanillaPlus] Root - IsMainThread: False
16:22:50.995 | DBG | [VanillaPlus] Task<T> - IsMainThread: True
16:22:50.996 | DBG | [VanillaPlus] Root - IsMainThread: False
16:22:51.499 | DBG | [VanillaPlus] Task - IsMainThread: True
16:22:51.499 | DBG | [VanillaPlus] Root - IsMainThread: False
16:22:51.499 | INF | [VanillaPlus] Successfully Enabled DebugGameModification
16:22:51.499 | DBG | [VanillaPlus] Saving system.config.json

Testing using Run

    public override async Task OnEnableAsync() {
        IPluginLog.Get().Debug($"Root - IsMainThread: {ThreadSafety.IsMainThread}");

        await IFramework.Get().Run(async () => {
            IPluginLog.Get().Debug($"DeadlockTest - IsMainThread: {ThreadSafety.IsMainThread}");

            await IFramework.Get().Run(() => {
                IPluginLog.Get().Debug("DID WE DEADLOCK?");
                IPluginLog.Get().Debug($"DeadlockTest INNER - IsMainThread: {ThreadSafety.IsMainThread}");
            });

            IPluginLog.Get().Debug($"DeadlockTest Post-Nesting - IsMainThread: {ThreadSafety.IsMainThread}");
        });

        IPluginLog.Get().Debug($"Root - IsMainThread: {ThreadSafety.IsMainThread}");

        await IFramework.Get().Run(() => {
            IPluginLog.Get().Debug($"Action - IsMainThread: {ThreadSafety.IsMainThread}");
        });

        IPluginLog.Get().Debug($"Root - IsMainThread: {ThreadSafety.IsMainThread}");

        await IFramework.Get().Run(() => {
            IPluginLog.Get().Debug($"Func - IsMainThread: {ThreadSafety.IsMainThread}");
            return true;
        });

        IPluginLog.Get().Debug($"Root - IsMainThread: {ThreadSafety.IsMainThread}");

        await IFramework.Get().Run(async () => {
            IPluginLog.Get().Debug($"Task<T> - IsMainThread: {ThreadSafety.IsMainThread}");
            return true;
        });

        IPluginLog.Get().Debug($"Root - IsMainThread: {ThreadSafety.IsMainThread}");

        await IFramework.Get().Run(async () => {
            IPluginLog.Get().Debug($"Task - IsMainThread: {ThreadSafety.IsMainThread}");
        });

        IPluginLog.Get().Debug($"Root - IsMainThread: {ThreadSafety.IsMainThread}");
    }

16:31:51.424 | INF | [VanillaPlus] Enabling DebugGameModification
16:31:51.425 | DBG | [VanillaPlus] Root - IsMainThread: False
16:31:51.426 | DBG | [VanillaPlus] DeadlockTest - IsMainThread: True
16:31:51.426 | DBG | [VanillaPlus] DID WE DEADLOCK?
16:31:51.426 | DBG | [VanillaPlus] DeadlockTest INNER - IsMainThread: True
16:31:51.426 | DBG | [VanillaPlus] DeadlockTest Post-Nesting - IsMainThread: True
16:31:51.426 | DBG | [VanillaPlus] Root - IsMainThread: False
16:31:51.434 | DBG | [VanillaPlus] Action - IsMainThread: True
16:31:51.434 | DBG | [VanillaPlus] Root - IsMainThread: False
16:31:51.442 | DBG | [VanillaPlus] Func - IsMainThread: True
16:31:51.442 | DBG | [VanillaPlus] Root - IsMainThread: False
16:31:51.451 | DBG | [VanillaPlus] Task<T> - IsMainThread: True
16:31:51.451 | DBG | [VanillaPlus] Root - IsMainThread: False
16:31:51.459 | DBG | [VanillaPlus] Task - IsMainThread: True
16:31:51.459 | DBG | [VanillaPlus] Root - IsMainThread: False
16:31:51.459 | INF | [VanillaPlus] Successfully Enabled DebugGameModification
16:31:51.460 | DBG | [VanillaPlus] Saving system.config.json

Demonstrates that all parts of these functions are operating as expected.

Misc: Fixes some IDE suggestions in UiBuilder to use backing fields.

Fixes various deadlock cases, and ensures awaits work as expected.

…esult to maintain non-blocking behavior.

# Conflicts: # Dalamud/Game/Framework.cs

goaaats · 2026-09-07T22:00:49Z

-            cancellationToken,
-            TaskContinuationOptions.HideScheduler,
-            this.frameworkThreadTaskScheduler).Unwrap();
+        await Task.Factory.StartNew(


Does this need a .Unwrap()? func is a functor that returns a Task, which you're explicitly not awaiting here, so the code people actually care about disappears into thin air and gets detached. The the generic overload above doesn't have this problem and it looks like the tests in the description don't catch it since they don't use await, I assume it's just an oversight. We should have a self-test that covers this tho, if it managed to slip through

You're correct, the original code had an unwrap here, that got missed when updating to the new approach.

goaaats · 2026-09-07T22:10:44Z

+
+        linkedCts.Token.ThrowIfCancellationRequested();
+
+        if (this.IsInFrameworkUpdateThread)


I think the fact that the two branches here act different now is pretty dangerous. The else branch hides the scheduler, so every await is back on the framework thread, but in the if branch doesn't do this, so the scheduler is preserved and you end up on the thread pool if you await. Is that what you were intending? The same call has two different threading behaviors based on where you call it from, so I think it might actually be more confusing

Maybe a good example is something like this:

await framework.Run(async () => { await SomeOperation(); SomethingWithATK(); // framework thread if called off-thread but thread pool if called on it });

The synchronous Action / Func overloads don't have this, since there's no continuation. So one option is to keep the inline shortcut only for those two and let the async overloads always go through the scheduler, maybe, but I don't know if that has problems during shutdown. Otherwise the inline path needs to run under the framework scheduler as well. We should also have a test for that.

Good catch, I hadn't considered the difference between passing an action/func vs passing a task here.

I have tested with removing the the IsInFrameworkUpdateThread shortcut, and there seems to be no issues with unloading that I found.

goaaats · 2026-09-07T22:11:29Z

+
+        linkedCts.Token.ThrowIfCancellationRequested();
+
+        if (this.IsInFrameworkUpdateThread)


This has the same problem as L181

Same as previous feedback, removed the shortcut and tested behavior.

goaaats · 2026-09-07T22:20:27Z


    /// <inheritdoc/>
-    public Task<T> RunOnTick<T>(Func<T> func, TimeSpan delay = default, int delayTicks = default, CancellationToken cancellationToken = default)
+    public async Task<T> RunOnTick<T>(Func<T> func, TimeSpan delay = default, int delayTicks = 0, CancellationToken cancellationToken = default)


I guess making these async means that if you call .Wait() or .Result on them on the framework thread you would deadlock for sure since you're awaiting something on the next tick from the current tick? Should probably be in the docs if that's intentional?

Tested that theory with:

await IFramework.Get().Run(() => { IFramework.Get().RunOnTick(() => IPluginLog.Get().Debug("Did we deadlock?")).Wait(); });

And it does definitely deadlock. I'll add some docs. Personally it does not make sense to use RunOnTick.Wait() logically you'd use Run.

Because I feel that it's invalid to say "I wanna do this next frame(or later), but I want to wait for it." in non-async context is weird.

goaaats · 2026-09-07T22:24:54Z

    /// version of <c>RunOnFrameworkThread</c>.</para>
    /// </remarks>
-    [Obsolete($"Use {nameof(RunOnTick)} instead.")]
+    [Obsolete($"Use {nameof(RunOnTick)} instead.", true)]


We use these still in Dalamud, right? Should we move? The one in Framework isn't obsolete so we don't get warnings inside Dalamud

The one in Framework not being marked Obsolete was an oversight. This PR already switched over a few calls that resolve it via the interface, and there were no issues with those. I will mark the Framework calls as Obsolete aswell, update dalamud internals, and test for side-effect. I don't anticipate any side effects.

I have updated all the calls to RunOnFrameworkThread in dalamud to use Run or RunOnTick, there were one or two cases where Run caused unexpected behavior, but those were things that get set inside delegates that get invoked through some complex mechanism that I wasn't able to track down, so those were switched to RunOnTick and seemed to behave properly.

goaaats · 2026-09-07T22:28:20Z


    /// <inheritdoc/>
-    public Task<T> RunOnTick<T>(Func<T> func, TimeSpan delay = default, int delayTicks = default, CancellationToken cancellationToken = default)
+    public async Task<T> RunOnTick<T>(Func<T> func, TimeSpan delay = default, int delayTicks = 0, CancellationToken cancellationToken = default)


Another thing, these return a cancelled task during shutdown now instead of just running in-place we we used to. Maybe that's fine (makes some sense to me since it's more correct, but less predictable for devs since they will not notice till they shut their game down and manage to look in the log after) but the calls in Dalamud still use the old functions that do the in-place run so I'm afraid that this is unexpected for devs. Intentional?

Dalamud unload logic was changed with Hasel's PR's for cleaner unload, so it's my understanding that frameworkDestroyed only occurs after all plugins have been unloaded.

Hasel's PR effectively made plugins now no longer need to know if the game is unloading, as the games tick is kept alive until all plugins have been unloaded/disposed.

KamiToolKit and VanillaPlus take significant advantage of this behavior. So I don't think devs will see any issues with things not running and throwing exceptions/task cancellations during unload.

I may be misunderstanding the issue here.

goaaats · 2026-09-07T22:30:41Z

    [ServiceManager.ServiceDependency]
    private readonly Framework frameworkService = Service<Framework>.Get();

+    private readonly CancellationTokenSource pluginUnloadCancellationToken;


Dispose? Also it's a token source not a token (but whatever)

I've renamed the variable, but I'm not sure what the proper way to dispose of a CTS, it feels improper to .Cancel then .Dispose? Unless that's fine?

goaaats · 2026-09-07T22:34:22Z

-        => this.frameworkService.RunOnTick(func, delay, delayTicks, cancellationToken);
+    public async Task<T> RunOnTick<T>(Func<T> func, TimeSpan delay = default, int delayTicks = 0, CancellationToken cancellationToken = default)
+    {
+        using var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, this.pluginUnloadCancellationToken.Token);


We have like triple-layered CTS flying around here now, maybe deserves a comment at the top of the file explaining the idea behind it. Also need to be very careful to dispose all of these since they're linked to the session-wide source and created for every call but it looks fine at a glance

Oh not sure why my comment to this one disappeared...

I did some digging on the linked token source, and it does not seem to mutate or effect the original tokens in any way, and every use of the CreateLinkedTokenSource is automatically disposed via using, so I think we're good here.

As for performance impacts, docs seemed to indicate that there's no meaningful impact for creating nested cancellation tokens.

The main intent with them is to have dalamud unload, plugin unload, and user cancellation all behave the same way.

goaaats · 2026-09-07T22:39:28Z

 /// This class represents the Framework of the native game client and grants access to various subsystems.
 /// </summary>
-/// <remarks>
-/// <para><b>Choosing between <c>RunOnFrameworkThread</c> and <c>Run</c></b></para>


I guess this still kind of matters in a way since there is still a difference in threading model between Run() and RunOnTick(). Might be worth explaining that here

I struggle with the particular nuance here, as both Run and RunOnTick will be operating on the main thread, what bearing does that have on a particular plugin dev other than that?

I understand that Run is using a task pool on the main thread, where as RunOnTick is running inside the games tick function, would the only meaningful difference be the timining of which Run vs RunOnTick occurs?

MidoriKami · 2026-09-08T04:47:36Z

Pushed updated Framework Fixes code, there's some some questions I have that I have posted to individual feedbacks.

I have tested dalamud loading and unloading with each individual change from RunOnFrameworkThread to Run/RunOnTick. The places where I use RunOnTick seemed to have issues with using Run and would prevent the game from loading.

It wasn't feezing/deadlocking the game, but it was leaving me with a black screen and often either a windows cursor, and sometimes a game cursor.

MidoriKami added 3 commits August 7, 2026 16:00

Framework service rework

78dabe1

Fixes various deadlock cases, and ensures awaits work as expected.

Stylecop/IDE suggestions

9305999

Use Framework.Run instead of RunOnFrameworkThread, and discard task r…

308c809

…esult to maintain non-blocking behavior.

MidoriKami requested a review from a team as a code owner August 7, 2026 23:24

MidoriKami and others added 8 commits August 7, 2026 16:26

Restore accidentally changed line

b0d13ef

Add RunOnTick notes

c08fba3

Use linked cancellation and make Run also async.

18b1cdd

Merge branch 'master' into FrameworkAsync

320de72

Add plugin unload cancellation token

7f3a354

Merge branch 'master' into FrameworkAsync

aa25bfb

Merge branch 'master' into FrameworkAsync

1ebed9d

# Conflicts: # Dalamud/Game/Framework.cs

IDE suggestions

f5ddf43

goaaats reviewed Sep 7, 2026

View reviewed changes

MidoriKami added 2 commits September 7, 2026 18:41

Merge branch 'refs/heads/master' into FrameworkAsync

2e270d2

Address feedback comments

58d0025

MidoriKami added 2 commits September 8, 2026 12:10

Merge branch 'master' into FrameworkAsync

89a6af3

Merge branch 'master' into FrameworkAsync

1cd6293


		linkedCts.Token.ThrowIfCancellationRequested();

		if (this.IsInFrameworkUpdateThread)

Conversation

MidoriKami commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The Problem

IFramework.Run(...)

IFramework.RunOnTick(...)

IFramework.RunOnFrameworkThread

Cancellation Tokens

Testing

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MidoriKami commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

MidoriKami commented Aug 7, 2026 •

edited

Loading

MidoriKami commented Sep 8, 2026 •

edited

Loading