Re: help needed: hsqldb shutdown race condition, how to fix without deadlock

On Wed, Jun 03, 2015 at 02:19:04PM +0200, Lionel Elie Mamane wrote:
> It now probably becomes a question for Stephan. See below.
>
> On Tue, Jun 02, 2015 at 03:17:16PM +0200, Lionel Elie Mamane wrote:
> > On Tue, Jun 02, 2015 at 03:12:51PM +0200, Noel Grandin wrote:
>
> > > I would suggest something like this in ODriverDelegator::flushConnections():
>
> > >     take_mutex();
> > >     std::vector<Connection> tmp = m_aConnections;
> > >     release_mutex();
> > >     for (Connection conn : tmp)
> > >         conn.flush();
>
> > > so that the mutex is not held during the problematic phase
>
> > That makes sense. Will try that. Thanks!
> But let's take it from the other side...
>
> connectivity::hsqldb::ODriverDelegator::preCommit does:
>
>
> 654   Reference<XConnection> xConnection(i->first,UNO_QUERY);
> 655   if ( xConnection.is() )
> 656   {
> 657         Reference< XStatement> xStmt = xConnection->createStatement();

> Now, this goes through a whole rigmarole:

> #6  0x00002abc1b6e90d3 in AffineBridge::v_callInto_v (this=0x2d29b50, pCallee=0x2abbf94eedbe 
> <s_pull(va_list*)>, 
>     pParam=0x2abc0fd9bdb0)
>     at 
> /home/master/src/libreoffice/workdirs/libreoffice-5-1/cppu/source/AffineBridge/AffineBridge.cxx:250
(...)
> #18 0x00002abc1b8f0ccc in Proxy::dispatch (this=this@entry=0x3577660, 
>     pReturnTypeRef=pReturnTypeRef@entry=0x3023920, pParams=pParams@entry=0x0, 
> nParams=nParams@entry=0, 
>     pMemberType=pMemberType@entry=0x3436a40, pReturn=pReturn@entry=0x2abc0fd9c2f0, 
> pArgs=0x2abc0fd9c2e0, 
>     ppException=0x2abc0fd9c390)
>     at 
> /home/master/src/libreoffice/workdirs/libreoffice-5-1/cppu/source/helper/purpenv/helper_purpenv_Proxy.cxx:445
(...)
> #21 0x00002abc0f3f2a6a in cpp_vtable_call (nFunctionIndex=<optimized out>, nVtableOffset=0, 
>     gpreg=0x2abc0fd9c710, fpreg=0x2abc0fd9c740, ovrflw=0x2abc0fd9c790, 
> pRegisterReturn=0x2abc0fd9c6f0)
>     at 
> /home/master/src/libreoffice/workdirs/libreoffice-5-1/bridges/source/cpp_uno/gcc3_linux_x86-64/cpp2uno.cxx:377
> #22 0x00002abc0f409c12 in privateSnippetExecutor ()
>    from /home/master/src/libreoffice/workdirs/libreoffice-5-1/instdir/program/libgcc3_uno.so
> #23 0x00002abc1b1a9fd8 in connectivity::hsqldb::ODriverDelegator::preCommit (this=0x2d24ef0, 
> aEvent=...)
>     at 
> /home/master/src/libreoffice/workdirs/libreoffice-5-1/connectivity/source/drivers/hsqldb/HDriver.cxx:657
> #24 0x00002abc18bb7081 in OStorage::BroadcastTransaction (this=this@entry=0x3578030, 

> The problem is essentially the serializing that happens at
> AffineBridge::v_callInto_v.

> Now, I guess that the whole privateSnippetExecutor / cpp_vtable_call /
> cpp2uno_call / s_Proxy_dispatch / ... / AffineBridge::v_callInto_v for
> a C++-to-C++ call is somehow linked to the fact that the calling code
> does not know the object that implements the XConnection
> interface. Can I avoid it in some way? For example, I *know* that
> m_xConnection is a connectivity::java_sql_connection.

> If I use that fact, can I avoid going through AffineBridge::v_callInto_v ?

I tried to do that in the attached patch, but as I was explained on
IRC, because the jdbc driver is in an affine component, that does not
work.

-- 
Lionel

diff --git a/connectivity/Library_hsqldb.mk b/connectivity/Library_hsqldb.mk
index 5fd647c..b97590b 100644
--- a/connectivity/Library_hsqldb.mk
+++ b/connectivity/Library_hsqldb.mk
@@ -26,6 +26,7 @@ $(eval $(call gb_Library_use_libraries,hsqldb,\
        cppu \
        cppuhelper \
        dbtools \
+       jdbc \
        jvmfwk \
        sal \
        salhelper \
diff --git a/connectivity/source/drivers/hsqldb/HDriver.cxx 
b/connectivity/source/drivers/hsqldb/HDriver.cxx
index 87eb271..b533819 100644
--- a/connectivity/source/drivers/hsqldb/HDriver.cxx
+++ b/connectivity/source/drivers/hsqldb/HDriver.cxx
@@ -21,6 +21,7 @@
 
 #include "hsqldb/HDriver.hxx"
 #include "hsqldb/HConnection.hxx"
+#include "java/sql/Connection.hxx"
 #include <osl/diagnose.h>
 #include <connectivity/dbexception.hxx>
 #include <com/sun/star/configuration/theDefaultProvider.hpp>
@@ -613,8 +614,14 @@ namespace connectivity
 
     void ODriverDelegator::flushConnections()
     {
-        TWeakPairVector::iterator aEnd = m_aConnections.end();
-        for (TWeakPairVector::iterator i = m_aConnections.begin(); aEnd != i; ++i)
+        TWeakPairVector connections;
+        {
+            ::osl::MutexGuard aGuard(m_aMutex);
+            connections = m_aConnections;
+        }
+
+        TWeakPairVector::const_iterator aEnd = connections.end();
+        for (TWeakPairVector::iterator i = connections.begin(); aEnd != i; ++i)
         {
             try
             {
@@ -648,15 +655,17 @@ namespace connectivity
                     Reference<XConnection> xConnection(i->first,UNO_QUERY);
                     if ( xConnection.is() )
                     {
-                        Reference< XStatement> xStmt = xConnection->createStatement();
+                        rtl::Reference<java_sql_Connection> 
pConnection(dynamic_cast<java_sql_Connection*>(xConnection.get()));
+                        assert(pConnection.is());
+                        Reference< XStatement> xStmt = pConnection->createStatement();
                         OSL_ENSURE( xStmt.is(), "ODriverDelegator::preCommit: no statement!" );
                         if ( xStmt.is() )
                             xStmt->execute( OUString(  "SET WRITE_DELAY 0"  ) );
 
-                        bool bPreviousAutoCommit = xConnection->getAutoCommit();
-                        xConnection->setAutoCommit( sal_False );
-                        xConnection->commit();
-                        xConnection->setAutoCommit( bPreviousAutoCommit );
+                        bool bPreviousAutoCommit = pConnection->getAutoCommit();
+                        pConnection->setAutoCommit( sal_False );
+                        pConnection->commit();
+                        pConnection->setAutoCommit( bPreviousAutoCommit );
 
                         if ( xStmt.is() )
                             xStmt->execute( OUString(  "SET WRITE_DELAY 60"  ) );
diff --git a/connectivity/source/inc/java/sql/Connection.hxx 
b/connectivity/source/inc/java/sql/Connection.hxx
index 47281e4..c521d95 100644
--- a/connectivity/source/inc/java/sql/Connection.hxx
+++ b/connectivity/source/inc/java/sql/Connection.hxx
@@ -37,7 +37,7 @@ namespace connectivity
 
     typedef OMetaConnection     java_sql_Connection_BASE;
 
-    class java_sql_Connection : public java_sql_Connection_BASE,
+    class SAL_DLLPUBLIC_EXPORT java_sql_Connection : public java_sql_Connection_BASE,
                                 public java_lang_Object,
                                 public OSubComponent<java_sql_Connection, 
java_sql_Connection_BASE>,
                                 public OAutoRetrievingBase
diff --git a/dbaccess/JunitTest_dbaccess_complex.mk b/dbaccess/JunitTest_dbaccess_complex.mk
index e3f2c00..913d7aa 100644
--- a/dbaccess/JunitTest_dbaccess_complex.mk
+++ b/dbaccess/JunitTest_dbaccess_complex.mk
@@ -25,8 +25,7 @@ $(eval $(call gb_JunitTest_set_defs,dbaccess_complex,\
 ))
 
 $(eval $(call gb_JunitTest_add_classes,dbaccess_complex,\
-    complex.dbaccess.Beamer \
-    complex.dbaccess.PropertyBag \
+       complex.dbaccess.RowSet \
 ))
 
 $(eval $(call gb_JunitTest_add_sourcefiles,dbaccess_complex,\
diff --git a/dbaccess/qa/complex/dbaccess/RowSet.java b/dbaccess/qa/complex/dbaccess/RowSet.java
index 156776a..149f77c 100644
--- a/dbaccess/qa/complex/dbaccess/RowSet.java
+++ b/dbaccess/qa/complex/dbaccess/RowSet.java
@@ -207,23 +207,31 @@ public class RowSet extends TestCase
         System.out.println("testing testRowSet");
         createTestCase(true);
 
+        System.out.println("testing testRowSet sequential");
         // sequential positioning
         m_resultSet.beforeFirst();
         testSequentialPositining(m_resultSet, m_row);
 
+        System.out.println("testing testRowSet absolute");
         // absolute positioning
         testAbsolutePositioning(m_resultSet, m_row);
 
+        System.out.println("testing testRowSet modify");
         // position during modify
         testModifyPosition(m_resultSet, m_row);
 
+        System.out.println("testing testRowSet 3rd");
         // 3rd test
         test3(createClone(), m_resultSet);
+        System.out.println("testing testRowSet 4th");
         // 4th test
         test4(m_resultSet);
+        System.out.println("finished testing testRowSet 4th");
 
+        // System.out.println("testing testRowSet concurrent");
         // concurrent (multi threaded) access to the row set and its clones
-        testConcurrentAccess(m_resultSet);
+        // testConcurrentAccess(m_resultSet);
+        // System.out.println("finished testRowSet concurrent");
     }
 
 
@@ -400,7 +408,7 @@ public class RowSet extends TestCase
     }
 
 
-    @Test
+    // @Test
     public void testRowSetEvents() throws java.lang.Exception
     {
         System.out.println("testing RowSet Events");
@@ -756,7 +764,7 @@ public class RowSet extends TestCase
     /** checks whether deletions on the main RowSet properly interfere (or don't interfere) with 
the movement
      *  on a clone of the RowSet
      */
-    @Test
+    // @Test
     public void testCloneMovesPlusDeletions() throws SQLException, UnknownPropertyException, 
WrappedTargetException
     {
         createTestCase(true);
@@ -825,7 +833,7 @@ public class RowSet extends TestCase
     /** checks whether insertions on the main RowSet properly interfere (or don't interfere) with 
the movement
      *  on a clone of the RowSet
      */
-    @Test
+    // @Test
     public void testCloneMovesPlusInsertions() throws SQLException, UnknownPropertyException, 
WrappedTargetException, PropertyVetoException, com.sun.star.lang.IllegalArgumentException
     {
         createTestCase(true);
@@ -1011,7 +1019,7 @@ public class RowSet extends TestCase
 
     /** checks the XParametersSupplier functionality of a RowSet
      */
-    @Test
+    // @Test
     public void testParameters()
     {
         createTestCase(false);