Treat CxPlatWorker as a full object
Description
Currently, if you have an app that is creating a datapath outside of msquic, that datapath always shares the same worker context as msquic. There are some cases where this is not the desired behavior, and instead a completely separate worker context should be used, such as if you need to guarantee there is always a free worker, and you have limited the msquic execution context with QUIC_PARAM_GLOBAL_EXECUTION_CONFIG.
This PR changes CxPlatWorker into a full object, with a default instance used. All the datapath init functions take a Worker Manager, and if that manager is null, the default one is used in its place.
Testing
Alll existing tests will implicitly hit this path, and cover any necessary testing.
Documentation
No documentation changes, as using CxPlat directly generally isn't a supported scenario.
@microsoft-github-policy-service agree
Generally looks good. Let's just rename WorkerManager to WorkerPool. I think I like that better. Thanks!
Codecov Report
Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
Project coverage is 84.69%. Comparing base (
22b7dda) to head (87628f7). Report is 1 commits behind head on main.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| src/core/library.c | 66.66% | 1 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #4492 +/- ##
==========================================
- Coverage 86.37% 84.69% -1.68%
==========================================
Files 56 56
Lines 15515 15520 +5
==========================================
- Hits 13401 13145 -256
- Misses 2114 2375 +261
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
core.lib(library.obj) : error LNK2001: unresolved external symbol CxPlatWorkerPoolInit [D:\a\msquic\msquic\src\bin\winkernel\msquicpriv.kernel.vcxproj] core.lib(library.obj) : error LNK2001: unresolved external symbol CxPlatWorkerPoolUninit [D:\a\msquic\msquic\src\bin\winkernel\msquicpriv.kernel.vcxproj]
Kernel mode doesn't currently rely on the worker pool.
core.lib(library.obj) : error LNK2001: unresolved external symbol CxPlatWorkerPoolInit [D:\a\msquic\msquic\src\bin\winkernel\msquicpriv.kernel.vcxproj] core.lib(library.obj) : error LNK2001: unresolved external symbol CxPlatWorkerPoolUninit [D:\a\msquic\msquic\src\bin\winkernel\msquicpriv.kernel.vcxproj]Kernel mode doesn't currently rely on the worker pool.
Fixed. Just made them a no-op in kernel mode. Should be fine, any use of CxPlatAddExecutionContext will assert, and then if any functions are used in the data paths they'll get linker errors.
Ok. Should now correctly be fixed. Forgot how UNREFERENCED_PARAMETER macro worked.
"D:\a\msquic\msquic\src\perf\lib\perflib.kernel.vcxproj" (default target) (9) ->
D:\a\msquic\msquic\src\inc\quic_platform.h(440,22): error C2065: 'WorkerPool': undeclared identifier [D:\a\msquic\msquic\src\perf\lib\perflib.kernel.vcxproj]
D:\a\msquic\msquic\src\inc\quic_platform.h(440,34): error C2059: syntax error: '(' [D:\a\msquic\msquic\src\perf\lib\perflib.kernel.vcxproj]
D:\a\msquic\msquic\src\inc\quic_datapath.h(408,14): error C2061: syntax error: identifier 'QUIC_EXECUTION_CONFIG' [D:\a\msquic\msquic\src\perf\lib\perflib.kernel.vcxproj]
D:\a\msquic\msquic\src\inc\quic_datapath.h(428,10): error C2061: syntax error: identifier 'QUIC_EXECUTION_CONFIG' [D:\a\msquic\msquic\src\perf\lib\perflib.kernel.vcxproj]
D:\a\msquic\msquic\src\inc\quic_platform.h(440,22): error C2065: 'WorkerPool': undeclared identifier [D:\a\msquic\msquic\src\perf\lib\perflib.kernel.vcxproj]
D:\a\msquic\msquic\src\inc\quic_platform.h(440,34): error C2059: syntax error: '(' [D:\a\msquic\msquic\src\perf\lib\perflib.kernel.vcxproj]
D:\a\msquic\msquic\src\inc\quic_platform.h(440,22): error C2065: 'WorkerPool': undeclared identifier [D:\a\msquic\msquic\src\perf\lib\perflib.kernel.vcxproj]
D:\a\msquic\msquic\src\inc\quic_platform.h(440,22): error C2065: 'WorkerPool': undeclared identifier [D:\a\msquic\msquic\src\perf\lib\perflib.kernel.vcxproj]
D:\a\msquic\msquic\src\inc\quic_datapath.h(408,14): error C2061: syntax error: identifier 'QUIC_EXECUTION_CONFIG' [D:\a\msquic\msquic\src\perf\lib\perflib.kernel.vcxproj]
D:\a\msquic\msquic\src\inc\quic_platform.h(440,34): error C2059: syntax error: '(' [D:\a\msquic\msquic\src\perf\lib\perflib.kernel.vcxproj]
D:\a\msquic\msquic\src\inc\quic_datapath.h(428,10): error C2061: syntax error: identifier 'QUIC_EXECUTION_CONFIG' [D:\a\msquic\msquic\src\perf\lib\perflib.kernel.vcxproj]
D:\a\msquic\msquic\src\inc\quic_platform.h(440,34): error C2059: syntax error: '(' [D:\a\msquic\msquic\src\perf\lib\perflib.kernel.vcxproj]
D:\a\msquic\msquic\src\inc\quic_datapath.h(408,14): error C2061: syntax error: identifier 'QUIC_EXECUTION_CONFIG' [D:\a\msquic\msquic\src\perf\lib\perflib.kernel.vcxproj]
D:\a\msquic\msquic\src\inc\quic_datapath.h(408,14): error C2061: syntax error: identifier 'QUIC_EXECUTION_CONFIG' [D:\a\msquic\msquic\src\perf\lib\perflib.kernel.vcxproj]
D:\a\msquic\msquic\src\inc\quic_datapath.h(428,10): error C2061: syntax error: identifier 'QUIC_EXECUTION_CONFIG' [D:\a\msquic\msquic\src\perf\lib\perflib.kernel.vcxproj]
D:\a\msquic\msquic\src\inc\quic_datapath.h(428,10): error C2061: syntax error: identifier 'QUIC_EXECUTION_CONFIG' [D:\a\msquic\msquic\src\perf\lib\perflib.kernel.vcxproj]
D:\a\msquic\msquic\src\perf\lib\SecNetPerfMain.cpp(230,5): error C2064: term does not evaluate to a function taking 1 arguments [D:\a\msquic\msquic\src\perf\lib\perflib.kernel.vcxproj]
D:\a\msquic\msquic\src\perf\lib\SecNetPerfMain.cpp(236,14): error C2660: 'CxPlatDataPathInitialize': function does not take 6 arguments [D:\a\msquic\msquic\src\perf\lib\perflib.kernel.vcxproj]
D:\a\msquic\msquic\src\perf\lib\SecNetPerfMain.cpp(238,9): error C3861: 'CxPlatWorkerPoolUninit': identifier not found [D:\a\msquic\msquic\src\perf\lib\perflib.kernel.vcxproj]
D:\a\msquic\msquic\src\perf\lib\SecNetPerfMain.cpp(282,9): error C3861: 'CxPlatWorkerPoolUninit': identifier not found [D:\a\msquic\msquic\src\perf\lib\perflib.kernel.vcxproj]
Looks like there are lots of test failures.
Some of those are failing on main after #4518 was merged. But there are a few that look to be me.
Some of those are failing on main after #4518 was merged. But there are a few that look to be me.
Crap, I thought everything was passing in that PR.