comphelper/source/misc/threadpool.cxx | 8 +++++++- package/qa/cppunit/test_package.cxx | 14 +++++++++++--- pyuno/source/module/pyuno_module.cxx | 21 ++++++++++++++++----- test/source/bootstrapfixture.cxx | 7 +++++++ test/source/vclbootstrapprotector.cxx | 3 +++ unotest/source/python/org/libreoffice/unotest.py | 3 +++ 6 files changed, 47 insertions(+), 9 deletions(-)
New commits: commit 9899ffd244dd367ba69dffe1f21f4f0222064a46 Author: Michael Stahl <mst...@redhat.com> Date: Sat Mar 11 00:44:21 2017 +0100 comphelper: fix MSVC hang in ThreadPool::shutdown() Commit aa68c99d88fd7abe08c4aee5206c859a0cdba38e added some code using std::condition_variable to comphelper. Built with MSVC 2017, this causes many cppunittester.exe processes to deadlock in ThreadPool::shutdown(): maTasksChanged.notify_all(); This ultimately calls NtReleaseKeyedEvent(), which never returns. The reason appears to be a bug in Windows 7, for which a "hotfix"[1] is avaiable here, but it's apparently not distributed via Windows Update so we likely can't rely on users or even developers having this installed. However, the documentation of DllMain[2] and ExitProcess[3] indicates that during shutdown, by the time global destructors are invoked all threads other than the one that called ExitProcess have already been terminated. Returning from main() implicitly calls ExitProcess [4]. As it turns out the problem only happens for some CppUnitTests because soffice.bin will call ThreadPool::shutdown() from Desktop::doShutdown() while it is still safe. [1] http://support.microsoft.com/kb/2582203 [2] https://msdn.microsoft.com/en-US/library/windows/desktop/ms682583(v=vs.85).aspx [3] https://msdn.microsoft.com/en-us/library/windows/desktop/ms682658(v=vs.85).aspx [4] https://blogs.msdn.microsoft.com/oldnewthing/20100827-00/?p=13023 Change-Id: I6137461ca7efe9a5fbe4f8f8478fb96de3570469 Reviewed-on: https://gerrit.libreoffice.org/35066 Tested-by: Jenkins <c...@libreoffice.org> Reviewed-by: Michael Stahl <mst...@redhat.com> diff --git a/comphelper/source/misc/threadpool.cxx b/comphelper/source/misc/threadpool.cxx index 712009d..1219ef2 100644 --- a/comphelper/source/misc/threadpool.cxx +++ b/comphelper/source/misc/threadpool.cxx @@ -89,7 +89,12 @@ ThreadPool::ThreadPool( sal_Int32 nWorkers ) : ThreadPool::~ThreadPool() { - shutdown(); + // note: calling shutdown from global variable dtor blocks forever on Win7 + // note2: there isn't enough MSVCRT left on exit to call assert() properly + // so these asserts just print something to stderr but exit status is + // still 0, but hopefully they will be more helpful on non-WNT platforms + assert(mbTerminate); + assert(maTasks.empty()); } struct ThreadPoolStatic : public rtl::StaticWithInit< std::shared_ptr< ThreadPool >, @@ -127,6 +132,7 @@ sal_Int32 ThreadPool::getPreferredConcurrency() return ThreadCount; } +// FIXME: what does "this" refer to in the following? // FIXME: there should be no need for this as/when our baseline // is >VS2015 and drop WinXP; the sorry details are here: // https://connect.microsoft.com/VisualStudio/feedback/details/1282596 diff --git a/package/qa/cppunit/test_package.cxx b/package/qa/cppunit/test_package.cxx index 335f490..d015e8d 100644 --- a/package/qa/cppunit/test_package.cxx +++ b/package/qa/cppunit/test_package.cxx @@ -119,8 +119,16 @@ namespace uno::Reference<container::XNameAccess> xNA(xZip, uno::UNO_QUERY); CPPUNIT_ASSERT(xNA.is()); + struct TestThreadPool { - comphelper::ThreadPool aPool(4); + comphelper::ThreadPool aPool; + TestThreadPool(sal_Int32 const i) : aPool(i) {} + ~TestThreadPool() { aPool.shutdown(); } + }; + + { + TestThreadPool aPool(4); + comphelper::ThreadPool & rPool(aPool.aPool); std::shared_ptr<comphelper::ThreadTaskTag> pTag = comphelper::ThreadPool::createThreadTaskTag(); std::vector<std::vector<char>> aTestBuffers(26); @@ -135,10 +143,10 @@ namespace xNA->getByName(aName) >>= xStrm; CPPUNIT_ASSERT(xStrm.is()); - aPool.pushTask(new Worker(pTag, xStrm, *itBuf)); + rPool.pushTask(new Worker(pTag, xStrm, *itBuf)); } - aPool.waitUntilDone(pTag); + rPool.waitUntilDone(pTag); // Verify the streams. CPPUNIT_ASSERT_EQUAL(size_t(26), aTestBuffers.size()); diff --git a/pyuno/source/module/pyuno_module.cxx b/pyuno/source/module/pyuno_module.cxx index 40ed69e..93a5811 100644 --- a/pyuno/source/module/pyuno_module.cxx +++ b/pyuno/source/module/pyuno_module.cxx @@ -318,7 +318,7 @@ static PyObject* getComponentContext( } static PyObject* initTestEnvironment( - SAL_UNUSED_PARAMETER PyObject*, SAL_UNUSED_PARAMETER PyObject*) + SAL_UNUSED_PARAMETER PyObject*, SAL_UNUSED_PARAMETER PyObject* args) { // this tries to bootstrap enough of the soffice from python to run // unit tests, which is only possible indirectly because pyuno is URE @@ -349,10 +349,21 @@ static PyObject* initTestEnvironment( mod.load(OStringToOUString(libname, osl_getThreadTextEncoding()), SAL_LOADMODULE_LAZY | SAL_LOADMODULE_GLOBAL); if (!mod.is()) { abort(); } - oslGenericFunction const pFunc( - mod.getFunctionSymbol("test_init")); - if (!pFunc) { abort(); } - reinterpret_cast<void (SAL_CALL *)(XMultiServiceFactory*)>(pFunc)(xMSF.get()); + assert(PyTuple_Check(args)); + if (PyTuple_Size(args) == 0) + { + oslGenericFunction const pFunc( + mod.getFunctionSymbol("test_init")); + if (!pFunc) { abort(); } + reinterpret_cast<void (SAL_CALL *)(XMultiServiceFactory*)>(pFunc)(xMSF.get()); + } + else + { + oslGenericFunction const pFunc( + mod.getFunctionSymbol("test_fini")); + if (!pFunc) { abort(); } + reinterpret_cast<void (SAL_CALL *)()>(pFunc)(); + } } catch (const css::uno::Exception &) { diff --git a/test/source/bootstrapfixture.cxx b/test/source/bootstrapfixture.cxx index c97c0ce..5fa01a3 100644 --- a/test/source/bootstrapfixture.cxx +++ b/test/source/bootstrapfixture.cxx @@ -16,6 +16,7 @@ #include <rtl/bootstrap.hxx> #include <cppuhelper/bootstrap.hxx> #include <comphelper/processfactory.hxx> +#include <comphelper/threadpool.hxx> #include <com/sun/star/lang/Locale.hpp> #include <com/sun/star/lang/XComponent.hpp> @@ -97,6 +98,12 @@ SAL_DLLPUBLIC_EXPORT void test_init(lang::XMultiServiceFactory *pFactory) catch (...) { abort(); } } +// this is called from pyuno +SAL_DLLPUBLIC_EXPORT void test_fini() +{ + ::comphelper::ThreadPool::getSharedOptimalPool().shutdown(); +} + } // extern "C" void test::BootstrapFixture::setUp() diff --git a/test/source/vclbootstrapprotector.cxx b/test/source/vclbootstrapprotector.cxx index 38c51d9..2218c8f 100644 --- a/test/source/vclbootstrapprotector.cxx +++ b/test/source/vclbootstrapprotector.cxx @@ -14,6 +14,7 @@ #include <sal/types.h> #include <test/setupvcl.hxx> #include <vcl/svapp.hxx> +#include <comphelper/threadpool.hxx> #include <isheadless.hxx> @@ -28,6 +29,8 @@ public: private: virtual ~Protector() override { DeInitVCL(); + // for the 6 tests that use it + comphelper::ThreadPool::getSharedOptimalPool().shutdown(); } virtual bool protect( diff --git a/unotest/source/python/org/libreoffice/unotest.py b/unotest/source/python/org/libreoffice/unotest.py index 13b0084..38c6b22 100644 --- a/unotest/source/python/org/libreoffice/unotest.py +++ b/unotest/source/python/org/libreoffice/unotest.py @@ -182,6 +182,9 @@ class UnoInProcess: global havePonies if not(havePonies): pyuno.private_initTestEnvironment() + # note: this will be called early enough, from Py_Finalize + import atexit + atexit.register(pyuno.private_initTestEnvironment, False) havePonies = True def openEmptyWriterDoc(self): assert(self.xContext) _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits