Make sure that the signing of .js and .vbs files with the AzureSignToolSigner uses STA context.#881
Make sure that the signing of .js and .vbs files with the AzureSignToolSigner uses STA context.#881dlemstra wants to merge 2 commits intodotnet:mainfrom
Conversation
…olSigner uses STA context.
dlemstra
force-pushed
the
js-vbs-in-sta-thread
branch
from
a71f3e7 to
4504884
Compare
|
|
||
| // loop through all of the files here, looking for appx/eappx | ||
| // mark each as being signed and strip appx | ||
| await Parallel.ForEachAsync(files, async (file, state) => |
There was a problem hiding this comment.
I tested the fix by signing a single .js file. The STA signing above succeeded, but this failed because it tries to sign all files on an MTA thread, which was the original problem.
This statement should sign all files that don't have a file extension in StaThreadExtensions.
We should have test coverage.
| { | ||
| internal sealed class AzureSignToolSigner : IAzureSignToolDataFormatSigner | ||
| { | ||
| private static readonly string[] StaThreadExtensions = [".js", ".vbs"]; |
There was a problem hiding this comment.
Comparisons should be case-insensitive.
| private static readonly string[] StaThreadExtensions = [".js", ".vbs"]; | |
| private static readonly HashSet<string> StaThreadExtensions = new(StringComparer.OrdinalIgnoreCase) | |
| { | |
| ".js", | |
| ".vbs" | |
| }; |
| return false; | ||
| } | ||
|
|
||
| private static Task<bool> RunOnStaThread(Func<Task> func) |
There was a problem hiding this comment.
| private static Task<bool> RunOnStaThread(Func<Task> func) | |
| private static Task<bool> RunOnStaThreadAsync(Func<Task> func) |
| FileInfo[] staThreadFiles = [.. files.Where(file => StaThreadExtensions.Contains(file.Extension))]; | ||
| foreach (FileInfo file in staThreadFiles) | ||
| { | ||
| await RunOnStaThread(() => SignAsync(signer, file, options)); |
There was a problem hiding this comment.
This only works because the actual signing operation happens to execute synchronously before any await calls. SignAsync(...) does contain an await Thread.Delay(...), but it only executes on retry. With the current implementation, only the first try has any chance of success while subsequent retries are guaranteed to fail because execution will resume on the thread pool (MTA).
The proper fix involves setting a synchronization context on the STA thread so that all continuations resume on the STA thread.
https://devblogs.microsoft.com/dotnet/await-synchronizationcontext-and-console-apps/
https://devblogs.microsoft.com/dotnet/await-synchronizationcontext-and-console-apps-part-2/
2 participants
Fixes #880