The current Win32 implementation of osl_terminateProcess calls directly upon the Win32 TerminateProcess() API function. When I looked up this function on MSDN I was a bit surprised to note that this is an asynchronous function that returns immediately before the process actually fully terminates, and does not necessarily succeed cleanly. As we are not checking that the process actually fully terminates, this is a bit of a problem, albeit one I think may well be somewhat obscure and non-obvious. In fact, it is better that we call on TerminateProcess and use WaitForSingleObject until the process actually fully terminates. If the termination ends abnormally, then we should pass I more appropriate exit status. Dr Dobbs magazine, bless its ancient archive, details a "safe" version of TerminateProcess, which has been implemented it seems in dozens of projects already so it's basically at this point a pattern. The article can be found here: http://www.drdobbs.com/a-safer-alternative-to-terminateprocess/184416547 Thankfully, the flaws they point out are only valid due to the age of the article as the Windows 9x line of operating systems was still a thing way hack then :-)
I will fix this and submit a commit to gerrit for review.
I have submitted a code change to gerrit. Can a Windows developer please have a look at this? Seems to pass all the OSL process tests... https://gerrit.libreoffice.org/#/c/35071/
What is the perceived problem here? Looking at the Unix implementation of osl_terminateProcess (sal/osl/unx/process.cxx), it appears that osl_terminateProcess isn't intended to wait for the given process to actually terminate (use osl_joinProcess for that).
Chris Sherlock committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=7fafd1aea08ad036ef48f415db5df93df218bf6e tdf#106489 - Win32 version of osl_terminateProcess not "safe" It will be available in 5.4.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Hello, Is this bug fixed? If so, could you please close it as RESOLVED FIXED?
This is now resolved. Changes that were made: 1. If the PID is 0x0 then this can't be ended, so return an invalid status 2. We need to elevate access to the process handle to allow it to create a new thread in a potentially remote process, so we use DuplicateHandle to do so 3. The core change is to change from TerminateProcess() to ExitProcess(), I'm letting it timeout forever and the end user can kill off the process via a Windows process management tool if they need it killed entirely, hence WaitForSingleObject() has an infinite timeout. This way, the termination handler is now called and the attached dlls are notified that they are being detached from the process.