[SYCL]: Suppress modal error message box on Windows #3493
Conversation
sycl/source/detail/windows_pi.cpp
Outdated
| // This is crucial for graceful handling of plugins that couldn't be | ||
| // loaded, e.g. due to missing native run-times. | ||
| // | ||
| (void)SetErrorMode(SEM_FAILCRITICALERRORS); |
There was a problem hiding this comment.
I'm not sure we can do this since it modifies behavior of user application.
Could you please provide more information about the problems with handling plugins loading?
There was a problem hiding this comment.
Because of various reasons (e.g. missing native RT installation) and depending on system setting a modal message box is displayed to the user (and application freezes):
Moreover this is the recommended usage: https://docs.microsoft.com/en-us/windows/win32/api/errhandlingapi/nf-errhandlingapi-seterrormode:
Best practice is that all applications call the process-wide SetErrorMode function with a parameter of SEM_FAILCRITICALERRORS at startup. This is to prevent error mode dialogs from hanging the application.
To be less intrusive we could change the setting only for loading plugins, and restore back after plugins initialization.
Would you prefer that ?
There was a problem hiding this comment.
I think so, there still be risk that it can affect a user if they have additional threads doing something non-SYCL related.
Can we have a toggle to disable setting this mode?
There was a problem hiding this comment.
On the other hand if this is going to address hangs in automated testing, probably, it's better to update tests to set this mode. What happens with these errors when SetErrorMode(SEM_FAILCRITICALERRORS) is set?
The doc says:
Instead, the system sends the error to the calling process.
But it is not quite clear which function gets the error.
There was a problem hiding this comment.
Can we have a toggle to disable setting this mode?
I think we absolutely want to unconditionally suppress message boxes during plugins loading. If we choose to only affect plugins initialization (by restoring back users settings afterwards) then we don't need a toggle.
But it is not quite clear which function gets the error.
I think it would be through https://docs.microsoft.com/en-us/windows/win32/api/errhandlingapi/nf-errhandlingapi-getlasterror but I didn't try that (and wouldn't want to do it as a part of this PR).
There was a problem hiding this comment.
Could you please clarify what happens then if there is an error like missing ze_loader.dll if SetErrorMode(SEM_FAILCRITICALERRORS) is done?
There was a problem hiding this comment.
the LoadLibraryA returns NULL, and the Level-Zero plugin is not loaded, ignored
There was a problem hiding this comment.
@romanovvlad , do you need more clarifications? can we merge it?
Signed-off-by: Sergey V Maslov <[email protected]>
Signed-off-by: Sergey V Maslov [email protected]