avoid using LocalFree on FormatMessageW buffer #10796
Conversation
|
I think there are two conflict requirements:
I believe Onecoreuap.lib and Onecoreuap_apiset.lib play different roles here. Am I right? |
Yes, correct.
We are trying to produce binaries that work both downlevel on desktop down to Win 8.1, and across WCOS devices. Onecoreuap.lib and onecoreuap_apiset.lib have the same API coverage. However Onecoreuap.lib will make it so that apiset dlls are not explicitly required and will fallback to legacy .dll's using Reverse Forwarders. https://docs.microsoft.com/en-us/windows/win32/apiindex/api-set-loader-operation#reverse-forwarding Right now with onecoreuap_apiset.lib, direct forwarding is used and the api set dll is attempted to be loaded directly and the application crashes if that apiset dll doesn't exist such as on 8.1. In this case, LocalFree is available on Kernel32.dll but it chooses to use api-ms-win-core-heap-l2-1-0.dll
Addressing your concern about validation, API Validator can be added to the pipeline so that it'll fail / emit warnings if API's are used in Onnxruntime.dll and Winml dll are breaking backwards compatibility : https://docs.microsoft.com/en-us/windows-hardware/drivers/develop/validating-windows-drivers#apivalidator Transitioning to using onecoreuap.lib + adding APIValidator into the pipeline may take some time. I think that work definitely needs to be done to make developer lives easier since we'll have pipeline level validation. However, since 1.11 release deadline is coming up, I would like to add this PR's change to unblock Windows 8.1 and in a separate PR update to use onecoreuap.lib + API validator. It is putting a temporary bandaid until a refactor is done. What do you think? |
|
ryanlai2, smk2007 and I had a discussion: for this release, we will keep it as what it was in the last release(1.10), that is we should avoid using LocalFree. And @ryanlai2 will use APIValidator to verify it. Going forward, we need try to make it simpler. Now it works because Tiago added some hacks. I suggested a single desktop binary should be enough for most use cases. For example, Windows python.exe is such a things. Python.exe links to kernel32.dll but it works fine on onecore and onecoreuap editions. I suggest onnxruntime does the same, and be aligned with what Visual Studio provides. onnxruntime should keep the wcos build option in build.py in case someone still wants a pure onecore build. |
* remove local free * Remove local free from onnxruntime * don't allocate * Change to use constexpr to satisfy CPU build warning
* Update to flatbuffers v2.0.0 (#10866) * Fix Reduced ops pipeline (#10861) * Fix a couple of issues with the python package tools (#10858) * Tweaks to the model utils * Add handling for a dim_value of -1 when replacing the entire input shape. This occurs in models exported from PaddlePaddle * make pytorch helpers accessible in package * make QDQ helpers accessible in package * Fix wrong percentile values returned during calibration (#10847) * Use numpy.percentile to get the lookup value. * Use 1.0 as float value rather than integer. * Add missing cdf parameter for `np.percentile`. * Use 100. instead of 1.0 * Remove print. * Update from @yufenglee * Add support for opset 16 to transpose optimizer. (#10841) * Add support for opset 16 to transpose optimizer. Only change required is for GridSample to be added to the layout sensitive ops. The existing handling for layout transpose works with that as the first input and first output are layout sensitive. Update the optimize to be able to return an error message if it fails. * Use separate build directories for full and mobile iOS packages. (#10835) * Address performance issue with abseil flat_hash_table. (#10819) When returning by value in a cross DLL call, the hash table even though containing all the entries that are originally there can not find at least some of them. Reverting to std::unordered_set pending further investigation. * Mark end of version 11 C API. (#10803) * Mark end of version 11 C API * Add static_assert * avoid using LocalFree on FormatMessageW buffer (#10796) * remove local free * Remove local free from onnxruntime * don't allocate * Change to use constexpr to satisfy CPU build warning * Integrate C-API tests into Pipelines for release packages (#10794) * add c-api test for package * fix bug for running c-api test for package * refine run application script * remove redundant code * include CUDA test * Remove testing CUDA EP temporarily * fix bug * Code refactor * try to fix YAML bug * try to fix YAML bug * try to fix YAML bug * fix bug for multiple directories in Pipelines * fix bug * add comments and fix bug * Update c-api-noopenmp-packaging-pipelines.yml * Remove failOnStandardError flag in Pipelines * Detect runtime CUDA JIT and warn the user (#10781) * Use cudaMalloc vs cudaDeviceSynchronize and show the total time * Update convert_onnx_models_to_ort.py to support runtime optimizations. (#10765) Add runtime optimization support to ONNX -> ORT format conversion script. Replace `--optimization_level`, `--use_nnapi`, and `--use_coreml` with a new `--optimization_style` option. * Add multithreading test and put a lock on nvinfer1::createInferRuntime() for TRT EP (#10714) * Add multithread unit test and put lock on library call * update code * remove debug code * add comment * add one session multi-threads inference * Put lock for build engine all the time * Update naming and comment * remove unnecessary lock * Revert "remove unnecessary lock" This reverts commit 9c2317b. * Fix handling of nodes inserted by NHWC transformer. (#10904) (#10925) * Revert "Upsample support NHWC (#10554)" (#10917) This reverts commit bd08f11. Co-authored-by: Yufeng Li <liyufeng1987@gmail.com> * [python API] Change raise import error when `C:\Windows\System32\vcruntime140_1.dll` is not found to warning (#10927) * remove throw if C:\\Windows\\System32\\vcruntime140_1.dll cannot be found * Add comments and update warning message * adding back accidentally removed line Co-authored-by: gwang0000 <62914304+gwang0000@users.noreply.github.com> * [js] Create npm packaging pipeline (#10886) * create npm packaging pipeline * fix indentations * Update npm-packaging-pipeline.yml for Azure Pipelines * Update npm-packaging-pipeline.yml for Azure Pipelines * Update npm-packaging-pipeline.yml for Azure Pipelines * react-native-ci as a template * fix typos * fix template paths * add a depencendy * change a stage name * set different artifact name for each package * fix typo * Update npm-packaging-pipeline.yml for Azure Pipelines Set a build Id for node npm package as a parameter * Update npm-packaging-pipeline.yml for Azure Pipelines Set a build Id for node npm package as a parameter * Update npm-packaging-pipeline.yml for Azure Pipelines * Follow up update for python API checking if `vcruntime140_1.dll` is available (#10927) (#10933) Co-authored-by: Hariharan Seshadri <hasesh@microsoft.com> Co-authored-by: Scott McKay <skottmckay@gmail.com> Co-authored-by: Funtowicz Morgan <mfuntowicz@users.noreply.github.com> Co-authored-by: Edward Chen <18449977+edgchen1@users.noreply.github.com> Co-authored-by: Dmitri Smirnov <yuslepukhin@users.noreply.github.com> Co-authored-by: Pranav Sharma <prs@microsoft.com> Co-authored-by: Ryan Lai <rylai@microsoft.com> Co-authored-by: Ryan Hill <38674843+RyanUnderhill@users.noreply.github.com> Co-authored-by: Yi-Hong Lyu <yilyu@microsoft.com> Co-authored-by: Yufeng Li <liyufeng1987@gmail.com> Co-authored-by: Guoyu Wang <62914304+gwang-msft@users.noreply.github.com> Co-authored-by: gwang0000 <62914304+gwang0000@users.noreply.github.com> Co-authored-by: Sunghoon <35605090+hanbitmyths@users.noreply.github.com>
* remove local free * Remove local free from onnxruntime * don't allocate * Change to use constexpr to satisfy CPU build warning
|
You can implement it yourself to support Windows 7. The issue is resolved here: |
LocalFree doesn't work downlevel on windows 8.1.
Onecoreuap.lib should be used so that specific apisets don't need to be required to run on downlevel OS's like 8.1 and can fall back to using kernel32.dll using reverseforwarders.
The builds need to be changed in a separate change, from using Onecoreuap_apiset.lib to using onecoreuap.lib. With this change, a check must be added in the CI pipelines by using the APIValidator tool to check for regressions