ucb/source/ucp/file/filrow.cxx | 1 ucb/source/ucp/file/filrset.cxx | 63 +++++++++++++++++++--------------------- ucb/source/ucp/file/filrset.hxx | 4 +- 3 files changed, 33 insertions(+), 35 deletions(-)
New commits: commit 12e7a664f8e0accea5b52d9537d8133a5e7a2c2c Author: Noel Grandin <[email protected]> AuthorDate: Wed Oct 2 19:19:20 2024 +0200 Commit: Noel Grandin <[email protected]> CommitDate: Thu Oct 3 10:39:38 2024 +0200 cid#1554974 Data race condition Change-Id: I9b9b1bb9f63b9eaf294b27afbde4c39e773ede8b Reviewed-on: https://gerrit.libreoffice.org/c/core/+/174406 Tested-by: Jenkins Reviewed-by: Noel Grandin <[email protected]> diff --git a/ucb/source/ucp/file/filrow.cxx b/ucb/source/ucp/file/filrow.cxx index e352e7919907..066233aafc2f 100644 --- a/ucb/source/ucp/file/filrow.cxx +++ b/ucb/source/ucp/file/filrow.cxx @@ -90,6 +90,7 @@ XRow_impl::~XRow_impl() sal_Bool SAL_CALL XRow_impl::wasNull() { + std::scoped_lock aGuard( m_aMutex ); return m_nWasNull; } commit 74ed4d1937449009343676546a937420a1ef5570 Author: Noel Grandin <[email protected]> AuthorDate: Wed Oct 2 19:15:05 2024 +0200 Commit: Noel Grandin <[email protected]> CommitDate: Thu Oct 3 10:39:27 2024 +0200 cid#1608230 Data race condition and cid#1607831 Data race condition cid#1607761 Check of thread-shared field evades lock acquisition cid#1606784 Data race condition Change-Id: Icbc872531117fbd9c22e33ffc560e07322f39ef8 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/174405 Tested-by: Jenkins Reviewed-by: Noel Grandin <[email protected]> diff --git a/ucb/source/ucp/file/filrset.cxx b/ucb/source/ucp/file/filrset.cxx index 5aa3007d7b4d..7c911d57d6b0 100644 --- a/ucb/source/ucp/file/filrset.cxx +++ b/ucb/source/ucp/file/filrset.cxx @@ -128,42 +128,33 @@ XResultSet_impl::dispose() void XResultSet_impl::rowCountChanged(std::unique_lock<std::mutex>& rGuard) { - sal_Int32 aOldValue,aNewValue; - std::vector< uno::Reference< beans::XPropertyChangeListener > > seq = m_aRowCountListeners.getElements(rGuard); - aNewValue = m_aItems.size(); - aOldValue = aNewValue-1; + sal_Int32 aNewValue = m_aItems.size(); + sal_Int32 aOldValue = aNewValue-1; beans::PropertyChangeEvent aEv; aEv.PropertyName = "RowCount"; aEv.Further = false; aEv.PropertyHandle = -1; aEv.OldValue <<= aOldValue; aEv.NewValue <<= aNewValue; - for( const auto& listener : seq ) - listener->propertyChange( aEv ); + m_aRowCountListeners.notifyEach(rGuard, &beans::XPropertyChangeListener::propertyChange, aEv); } -void XResultSet_impl::isFinalChanged() +void XResultSet_impl::isFinalChanged(std::unique_lock<std::mutex>& rGuard) { - std::vector< uno::Reference< beans::XPropertyChangeListener > > seq; - { - std::unique_lock aGuard( m_aMutex ); - seq = m_aIsFinalListeners.getElements(aGuard); - m_bRowCountFinal = true; - } + m_bRowCountFinal = true; beans::PropertyChangeEvent aEv; aEv.PropertyName = "IsRowCountFinal"; aEv.Further = false; aEv.PropertyHandle = -1; aEv.OldValue <<= false; aEv.NewValue <<= true; - for( const auto& listener : seq ) - listener->propertyChange( aEv ); + m_aIsFinalListeners.notifyEach(rGuard, &beans::XPropertyChangeListener::propertyChange, aEv); } bool -XResultSet_impl::OneMore() +XResultSet_impl::OneMore(std::unique_lock<std::mutex>& rGuard) { if( ! m_nIsOpen ) return false; @@ -181,7 +172,7 @@ XResultSet_impl::OneMore() if( err == osl::FileBase::E_NOENT || err == osl::FileBase::E_INVAL ) { m_aFolder.close(); - isFinalChanged(); + isFinalChanged(rGuard); m_nIsOpen = false; return m_nIsOpen; } @@ -197,11 +188,10 @@ XResultSet_impl::OneMore() if( m_nOpenMode == ucb::OpenMode::DOCUMENTS && IsRegular ) { - std::unique_lock aGuard( m_aMutex ); m_aItems.push_back( aRow ); m_aIdents.emplace_back( ); m_aUnqPath.push_back( aUnqPath ); - rowCountChanged(aGuard); + rowCountChanged(rGuard); return true; } @@ -211,11 +201,10 @@ XResultSet_impl::OneMore() } else if( m_nOpenMode == ucb::OpenMode::FOLDERS && ! IsRegular ) { - std::unique_lock aGuard( m_aMutex ); m_aItems.push_back( aRow ); m_aIdents.emplace_back( ); m_aUnqPath.push_back( aUnqPath ); - rowCountChanged(aGuard); + rowCountChanged(rGuard); return true; } else if( m_nOpenMode == ucb::OpenMode::FOLDERS && IsRegular ) @@ -224,11 +213,10 @@ XResultSet_impl::OneMore() } else { - std::unique_lock aGuard( m_aMutex ); m_aItems.push_back( aRow ); m_aIdents.emplace_back( ); m_aUnqPath.push_back( aUnqPath ); - rowCountChanged(aGuard); + rowCountChanged(rGuard); return true; } } @@ -243,10 +231,11 @@ XResultSet_impl::OneMore() sal_Bool SAL_CALL XResultSet_impl::next() { + std::unique_lock aGuard( m_aMutex ); bool test; if( ++m_nRow < sal::static_int_cast<sal_Int32>(m_aItems.size()) ) test = true; else - test = OneMore(); + test = OneMore(aGuard); return test; } @@ -275,8 +264,9 @@ XResultSet_impl::isFirst() sal_Bool SAL_CALL XResultSet_impl::isLast() { + std::unique_lock aGuard( m_aMutex ); if( m_nRow == sal::static_int_cast<sal_Int32>(m_aItems.size()) - 1 ) - return ! OneMore(); + return ! OneMore(aGuard); else return false; } @@ -292,8 +282,9 @@ XResultSet_impl::beforeFirst() void SAL_CALL XResultSet_impl::afterLast() { + std::unique_lock aGuard( m_aMutex ); m_nRow = sal::static_int_cast<sal_Int32>(m_aItems.size()); - while( OneMore() ) + while( OneMore(aGuard) ) ++m_nRow; } @@ -309,8 +300,9 @@ XResultSet_impl::first() sal_Bool SAL_CALL XResultSet_impl::last() { + std::unique_lock aGuard( m_aMutex ); m_nRow = sal::static_int_cast<sal_Int32>(m_aItems.size()) - 1; - while( OneMore() ) + while( OneMore(aGuard) ) ++m_nRow; return true; } @@ -329,11 +321,12 @@ XResultSet_impl::getRow() sal_Bool SAL_CALL XResultSet_impl::absolute( sal_Int32 row ) { + std::unique_lock aGuard( m_aMutex ); if( row >= 0 ) { m_nRow = row - 1; if( row >= sal::static_int_cast<sal_Int32>(m_aItems.size()) ) - while( row-- && OneMore() ) + while( row-- && OneMore(aGuard) ) ; } else @@ -411,11 +404,11 @@ XResultSet_impl::getStatement() void SAL_CALL XResultSet_impl::close() { + std::unique_lock aGuard( m_aMutex ); if( m_nIsOpen ) { m_aFolder.close(); - isFinalChanged(); - std::unique_lock aGuard( m_aMutex ); + isFinalChanged(aGuard); m_nIsOpen = false; } } @@ -437,6 +430,7 @@ XResultSet_impl::queryContentIdentifierString() uno::Reference< ucb::XContentIdentifier > SAL_CALL XResultSet_impl::queryContentIdentifier() { + std::unique_lock aGuard( m_aMutex ); if( 0 <= m_nRow && m_nRow < sal::static_int_cast<sal_Int32>(m_aItems.size()) ) { if( ! m_aIdents[m_nRow].is() ) @@ -518,9 +512,11 @@ void SAL_CALL XResultSet_impl::connectToCache( const uno::Reference< ucb::XDynamicResultSet > & xCache ) { - if( m_xListener.is() ) - throw ucb::ListenerAlreadySetException( THROW_WHERE ); - + { + std::unique_lock aGuard( m_aMutex ); + if( m_xListener.is() ) + throw ucb::ListenerAlreadySetException( THROW_WHERE ); + } uno::Reference< ucb::XSourceInitialization > xTarget( xCache, uno::UNO_QUERY ); if( xTarget.is() && m_pMyShell->m_xContext.is() ) @@ -608,6 +604,7 @@ void SAL_CALL XResultSet_impl::setPropertyValue( uno::Any SAL_CALL XResultSet_impl::getPropertyValue( const OUString& PropertyName ) { + std::unique_lock aGuard( m_aMutex ); if( PropertyName == "IsRowCountFinal" ) { return uno::Any(m_bRowCountFinal); diff --git a/ucb/source/ucp/file/filrset.hxx b/ucb/source/ucp/file/filrset.hxx index 8c44982a4861..4a25c16be723 100644 --- a/ucb/source/ucp/file/filrset.hxx +++ b/ucb/source/ucp/file/filrset.hxx @@ -421,10 +421,10 @@ class XResultSet_impl : // Methods /// @throws css::sdbc::SQLException /// @throws css::uno::RuntimeException - bool OneMore(); + bool OneMore(std::unique_lock<std::mutex>&); void rowCountChanged(std::unique_lock<std::mutex>&); - void isFinalChanged(); + void isFinalChanged(std::unique_lock<std::mutex>&); };
