Skip to content

COM server refactor and improvements - #537

Merged
Duncan Horn (dunhor) merged 6 commits into
microsoft:masterfrom
sylveon:master
Aug 6, 2025
Merged

COM server refactor and improvements#537
Duncan Horn (dunhor) merged 6 commits into
microsoft:masterfrom
sylveon:master

Conversation

@sylveon

@sylveonCharles Milette (sylveon) commented Jul 24, 2025

Copy link
Copy Markdown
Contributor

I originally only wanted to do the CLSID parameter change, but ended up spotting a lot of potential improvements:

  • Moved the class factory somewhere public because it can be useful for more things than just the COM server. Also made it so that the class factory only uses winrt::no_module_lock for usage in OOP servers like register_com_server because an in-proc class factory does need to increment the module lock.
  • Allow callers to alter the extra parameters of CoRegisterClassObject
  • Added reasons to the nodiscards
  • Made use of REGCLS_SUSPENDED in the overload that registers multiple instances
  • Use CoAddRefServerProcess and CoReleaseServerProcess in the lock we use - and also turn it purpose specific. This now means that as soon as the ref count hits 0, further activations are suspended to avoid potentially racy conditions
  • Now the COM server functionality lives in cppwinrt_authoring.h
  • Disabled weak references to the class factory, to mirror WRL.

I wanted to look into adding aggregation support to the factory, but backed off for now because cppwinrt's composition support machinery is almost entirely within the winrt::impl namespace and assumes IInspectable.

The end consumer API is almost identical. Most tests where modified very lightly if at all.

Supplants #533

@dunhor

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Comment threadinclude/wil/cppwinrt_authoring.h Outdated
@sylveon

Copy link
Copy Markdown
ContributorAuthor

I've pushed formatting fixes, however I am unsure what the errors on the clang build are about - they complain about illegal instructions but I don't appear to be doing anything that could trigger this.

@dunhor

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@dunhor

Copy link
Copy Markdown
Member

I am unsure what the errors on the clang build are about - they complain about illegal instructions but I don't appear to be doing anything that could trigger this.

Not sure about the illegal instruction part. I wonder if it hit an int 3 or something like that, but idk if that'll give the same error. Given the test it's failing in uses coroutines and I know there have been issues with Clang and coroutines on Windows in the past, I'm curious if that's related. If so, it's kind of disappointing those issues haven't been ironed out yet.

@dunhor

Copy link
Copy Markdown
Member

roxk as FYI for the changes in file names/types

@dunhor

Copy link
Copy Markdown
Member

I wonder if it hit an int 3 or something like that

If it is indeed an issue with coroutine support in Clang, it could also be a bad jump pointer, etc.

@sylveon

Copy link
Copy Markdown
ContributorAuthor

It's somehow hitting a compiler-generated ud2. When trying to call the AsyncActionCompletedHandler, i.e. after the coroutine is done running.

@sylveon

Charles Milette (sylveon) commented Jul 24, 2025

Copy link
Copy Markdown
ContributorAuthor

Swapping the (...) in our completed handler for the appropriate parameters fixes it. lol.

@sylveon

Copy link
Copy Markdown
ContributorAuthor

Should be fixed now.

@dunhor

Copy link
Copy Markdown
Member

/azp run

Comment threadinclude/wil/common.h Outdated
@dunhor

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@dunhor
Duncan Horn (dunhor) merged commit e778410 into microsoft:masterAug 6, 2025
11 checks passed
Fredrik Orderud (forderud) added a commit to forderud/ComSamples that referenced this pull request Jan 27, 2026
Sign up for freeto 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.

2 participants

@sylveon@dunhor