Skip to content

Make sure that the signing of .js and .vbs files with the AzureSignToolSigner uses STA context.#881

Merged
dtivel merged 5 commits intodotnet:mainfrom
dlemstra:js-vbs-in-sta-thread
Apr 20, 2026
Merged

Make sure that the signing of .js and .vbs files with the AzureSignToolSigner uses STA context.#881
dtivel merged 5 commits intodotnet:mainfrom
dlemstra:js-vbs-in-sta-thread

Conversation

@dlemstra
Copy link
Copy Markdown
Collaborator

Fixes #880

@dlemstra dlemstra requested a review from a team as a code owner May 23, 2025 16:01
@dlemstra dlemstra force-pushed the js-vbs-in-sta-thread branch from a71f3e7 to 4504884 Compare May 24, 2025 07:45
Comment thread src/Sign.Core/DataFormatSigners/AzureSignToolSigner.cs Outdated
Comment thread src/Sign.Core/DataFormatSigners/AzureSignToolSigner.cs Outdated
Comment thread src/Sign.Core/DataFormatSigners/AzureSignToolSigner.cs Outdated
Comment thread src/Sign.Core/DataFormatSigners/AzureSignToolSigner.cs Outdated
dtivel added a commit that referenced this pull request Mar 27, 2026
Address review concerns on PR #881:

- Run only the synchronous SignFile COM call on a dedicated STA thread
  (via RunOnStaThread<T>), not the entire async SignAsync. This avoids
  the need for a custom SynchronizationContext and ensures retries also
  run on STA threads.

- Partition STA-required files (.js, .vbs) from the Parallel.ForEachAsync
  loop. STA files are signed sequentially to avoid blocking ThreadPool
  threads via thread.Join(). Non-STA files continue to be signed in
  parallel as before.

- Use HashSet<string> with StringComparer.OrdinalIgnoreCase for
  case-insensitive extension matching.

- Unseal AzureSignToolSigner and extract SignFileCore as internal virtual
  to enable test substitution for apartment state verification.

- Add tests proving .vbs/.js files are signed on STA threads, .dll files
  on MTA threads, and mixed batches are correctly partitioned.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dtivel
Copy link
Copy Markdown
Collaborator

dtivel commented Mar 27, 2026

@dlemstra, I addressed the feedback in this commit.

Do you want to apply this commit and rebase on latest main?

@dlemstra dlemstra force-pushed the js-vbs-in-sta-thread branch from 4504884 to 4dec1fa Compare April 6, 2026 11:02
dlemstra pushed a commit to dlemstra/sign that referenced this pull request Apr 6, 2026
Address review concerns on PR dotnet#881:

- Run only the synchronous SignFile COM call on a dedicated STA thread
  (via RunOnStaThread<T>), not the entire async SignAsync. This avoids
  the need for a custom SynchronizationContext and ensures retries also
  run on STA threads.

- Partition STA-required files (.js, .vbs) from the Parallel.ForEachAsync
  loop. STA files are signed sequentially to avoid blocking ThreadPool
  threads via thread.Join(). Non-STA files continue to be signed in
  parallel as before.

- Use HashSet<string> with StringComparer.OrdinalIgnoreCase for
  case-insensitive extension matching.

- Unseal AzureSignToolSigner and extract SignFileCore as internal virtual
  to enable test substitution for apartment state verification.

- Add tests proving .vbs/.js files are signed on STA threads, .dll files
  on MTA threads, and mixed batches are correctly partitioned.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dlemstra
Copy link
Copy Markdown
Collaborator Author

dlemstra commented Apr 6, 2026

Thanks for helping out with this PR @dtivel! I have updated the pull request.

@dtivel
Copy link
Copy Markdown
Collaborator

dtivel commented Apr 16, 2026

@dlemstra, can you rebase on latest main one more time? The last build failed for some "infrastructure" issue.

dlemstra and others added 3 commits April 16, 2026 22:32
Address review concerns on PR dotnet#881:

- Run only the synchronous SignFile COM call on a dedicated STA thread
  (via RunOnStaThread<T>), not the entire async SignAsync. This avoids
  the need for a custom SynchronizationContext and ensures retries also
  run on STA threads.

- Partition STA-required files (.js, .vbs) from the Parallel.ForEachAsync
  loop. STA files are signed sequentially to avoid blocking ThreadPool
  threads via thread.Join(). Non-STA files continue to be signed in
  parallel as before.

- Use HashSet<string> with StringComparer.OrdinalIgnoreCase for
  case-insensitive extension matching.

- Unseal AzureSignToolSigner and extract SignFileCore as internal virtual
  to enable test substitution for apartment state verification.

- Add tests proving .vbs/.js files are signed on STA threads, .dll files
  on MTA threads, and mixed batches are correctly partitioned.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dlemstra dlemstra force-pushed the js-vbs-in-sta-thread branch from 4dec1fa to 04a1ad1 Compare April 16, 2026 20:32
@dtivel
Copy link
Copy Markdown
Collaborator

dtivel commented Apr 17, 2026

@dlemstra, it failed to build. I fixed the build error and 2 test issues in commit d8e9b97 .

@dlemstra
Copy link
Copy Markdown
Collaborator Author

Thanks for all the help again! It looks like we are ready to merge?

@dtivel dtivel merged commit 214b72b into dotnet:main Apr 20, 2026
3 checks passed
@dlemstra dlemstra deleted the js-vbs-in-sta-thread branch April 20, 2026 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

VBS files inside nupkg fail to sign

3 participants