[Libreoffice-commits] core.git: comphelper/source package/qa pyuno/source test/source unotest/source
Michael Stahl
mstahl at redhat.com
Sat Mar 11 10:09:01 UTC 2017
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 <mstahl at 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 <ci at libreoffice.org>
Reviewed-by: Michael Stahl <mstahl at 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)
More information about the Libreoffice-commits
mailing list