ucb/source/ucp/file/filrset.cxx | 10 +++- ucb/source/ucp/file/filtask.cxx | 88 +++++++++++++++++++++------------------- ucb/source/ucp/file/filtask.hxx | 5 +- 3 files changed, 59 insertions(+), 44 deletions(-)
New commits: commit f0110f798cee31ff87651dc2377eacef2ab8a8b7 Author: Stephan Bergmann <sberg...@redhat.com> Date: Wed Apr 19 13:58:59 2017 +0200 file UCP: Dir entries can disappear during non-atomic traversal ...so allow for that by reporting failure to call osl::DirectoryItem::getFileStatus from TaskManager::getv, and make XResultSet_impl::OneMore ignore such lost entries. While there may be legitimate cases where getFileStatus in getv would fail (and thus SAL_INFO would be more appropriate), the broken, non-atomic design means that such failure is likely unexpected (and worth a SAL_WARN). Occasionally ran into this when building UBSan builds, which sometimes failed in one of the extras/Gallery_*.mk like > ucb/source/ucp/file/filrset.cxx:235:60: runtime error: load of value 96, which is not a valid value for type 'bool' > #0 0x7f079bff575e in fileaccess::XResultSet_impl::OneMore() ucb/source/ucp/file/filrset.cxx:234:60 > #1 0x7f079bff823e in fileaccess::XResultSet_impl::next() ucb/source/ucp/file/filrset.cxx:288:16 > #2 0x7f0800a109a8 in Gallery::ImplLoadSubDirs(INetURLObject const&, bool&) svx/source/gallery2/gallery1.cxx:291:36 > #3 0x7f0800a0c88c in Gallery::ImplLoad(rtl::OUString const&) svx/source/gallery2/gallery1.cxx:202:5 > #4 0x7f0800a0bfa5 in Gallery::Gallery(rtl::OUString const&) svx/source/gallery2/gallery1.cxx:167:5 > #5 0x522e13 in createGallery(rtl::OUString const&) svx/source/gengal/gengal.cxx:62:16 > #6 0x52979a in createTheme(rtl::OUString const&, rtl::OUString const&, rtl::OUString const&, std::__debug::vector<INetURLObject, std::allocator<INetURLObject> >&, bool) svx/source/gengal/gengal.cxx:76:16 > #7 0x52853d in GalApp::Main() svx/source/gengal/gengal.cxx:318:9 > #8 0x7f080f6f8c36 in ImplSVMain() vcl/source/app/svmain.cxx:191:35 > #9 0x7f080f706571 in SVMain() vcl/source/app/svmain.cxx:229:16 > #10 0x56aa0a in sal_main() vcl/source/salmain/salmain.cxx:41:12 > #11 0x56a99f in main vcl/source/salmain/salmain.cxx:35:1 > #12 0x7f07fbaf5400 in __libc_start_main /usr/src/debug/glibc-2.24-33-ge9e69e4/csu/../csu/libc-start.c:289 > #13 0x42e219 in _start (instdir/program/gengal.bin+0x42e219) > > SUMMARY: AddressSanitizer: undefined-behavior ucb/source/ucp/file/filrset.cxx:235:60 in > solenv/gbuild/Gallery.mk:58: recipe for target 'workdir/Gallery/education.done' failed presumably because some other part of the build changed instdir/share/config/ in parallel. (And doing > while (touch instdir/share/config/TEST && rm instdir/share/config/TEST); do :; done in parallel to 'make extras' indeed causes this issue to occur easily.) Change-Id: I115ac727f99eed209b223d828c33060b275daaaa diff --git a/ucb/source/ucp/file/filrset.cxx b/ucb/source/ucp/file/filrset.cxx index 29e1b9124784..4cad6911abe2 100644 --- a/ucb/source/ucp/file/filrset.cxx +++ b/ucb/source/ucp/file/filrset.cxx @@ -228,8 +228,14 @@ XResultSet_impl::OneMore() } else if( err == osl::FileBase::E_None ) { - aRow = m_pMyShell->getv( - this, m_sProperty, aDirIte, aUnqPath, IsRegular ); + if (!m_pMyShell->getv( + this, m_sProperty, aDirIte, aUnqPath, IsRegular, aRow )) + { + SAL_WARN( + "ucb.ucp.file", + "getting dir item in <" << m_aBaseDirectory << "> failed"); + continue; + } if( m_nOpenMode == ucb::OpenMode::DOCUMENTS && IsRegular ) { diff --git a/ucb/source/ucp/file/filtask.cxx b/ucb/source/ucp/file/filtask.cxx index fbe93ed955f8..e78bb9ed46eb 100644 --- a/ucb/source/ucp/file/filtask.cxx +++ b/ucb/source/ucp/file/filtask.cxx @@ -2528,13 +2528,14 @@ TaskManager::commit( const TaskManager::ContentMap::iterator& it, // directoryitem, which is returned by osl::DirectoryItem::getNextItem() -uno::Reference< sdbc::XRow > SAL_CALL +bool SAL_CALL TaskManager::getv( Notifier* pNotifier, const uno::Sequence< beans::Property >& properties, osl::DirectoryItem& aDirItem, OUString& aUnqPath, - bool& aIsRegular ) + bool& aIsRegular, + uno::Reference< sdbc::XRow > & row ) { uno::Sequence< uno::Any > seq( properties.getLength() ); @@ -2548,57 +2549,64 @@ TaskManager::getv( osl_FileStatus_Mask_LinkTargetURL ); osl::FileBase::RC aRes = aDirItem.getFileStatus( aFileStatus ); - if ( aRes == osl::FileBase::E_None ) + if ( aRes != osl::FileBase::E_None ) { - aUnqPath = aFileStatus.getFileURL(); + SAL_WARN( + "ucb.ucp.file", + "osl::DirectoryItem::getFileStatus failed with " << +aRes); + return false; + } + + aUnqPath = aFileStatus.getFileURL(); - // If the directory item type is a link retrieve the type of the target + // If the directory item type is a link retrieve the type of the target - if ( aFileStatus.getFileType() == osl::FileStatus::Link ) + if ( aFileStatus.getFileType() == osl::FileStatus::Link ) + { + // Assume failure + aIsRegular = false; + osl::FileBase::RC result = osl::FileBase::E_INVAL; + osl::DirectoryItem aTargetItem; + osl::DirectoryItem::get( aFileStatus.getLinkTargetURL(), aTargetItem ); + if ( aTargetItem.is() ) { - // Assume failure - aIsRegular = false; - osl::FileBase::RC result = osl::FileBase::E_INVAL; - osl::DirectoryItem aTargetItem; - osl::DirectoryItem::get( aFileStatus.getLinkTargetURL(), aTargetItem ); - if ( aTargetItem.is() ) - { - osl::FileStatus aTargetStatus( osl_FileStatus_Mask_Type ); + osl::FileStatus aTargetStatus( osl_FileStatus_Mask_Type ); - if ( osl::FileBase::E_None == - ( result = aTargetItem.getFileStatus( aTargetStatus ) ) ) - aIsRegular = - aTargetStatus.getFileType() == osl::FileStatus::Regular; - } + if ( osl::FileBase::E_None == + ( result = aTargetItem.getFileStatus( aTargetStatus ) ) ) + aIsRegular = + aTargetStatus.getFileType() == osl::FileStatus::Regular; } - else - aIsRegular = aFileStatus.getFileType() == osl::FileStatus::Regular; + } + else + aIsRegular = aFileStatus.getFileType() == osl::FileStatus::Regular; - registerNotifier( aUnqPath,pNotifier ); - insertDefaultProperties( aUnqPath ); - { - osl::MutexGuard aGuard( m_aMutex ); + registerNotifier( aUnqPath,pNotifier ); + insertDefaultProperties( aUnqPath ); + { + osl::MutexGuard aGuard( m_aMutex ); - TaskManager::ContentMap::iterator it = m_aContent.find( aUnqPath ); - commit( it,aFileStatus ); + TaskManager::ContentMap::iterator it = m_aContent.find( aUnqPath ); + commit( it,aFileStatus ); - TaskManager::PropertySet::iterator it1; - PropertySet& propset = *(it->second.properties); + TaskManager::PropertySet::iterator it1; + PropertySet& propset = *(it->second.properties); - for( sal_Int32 i = 0; i < seq.getLength(); ++i ) - { - MyProperty readProp( properties[i].Name ); - it1 = propset.find( readProp ); - if( it1 == propset.end() ) - seq[i] = uno::Any(); - else - seq[i] = it1->getValue(); - } + for( sal_Int32 i = 0; i < seq.getLength(); ++i ) + { + MyProperty readProp( properties[i].Name ); + it1 = propset.find( readProp ); + if( it1 == propset.end() ) + seq[i] = uno::Any(); + else + seq[i] = it1->getValue(); } - deregisterNotifier( aUnqPath,pNotifier ); } + deregisterNotifier( aUnqPath,pNotifier ); + XRow_impl* p = new XRow_impl( this,seq ); - return uno::Reference< sdbc::XRow >( p ); + row = uno::Reference< sdbc::XRow >( p ); + return true; } diff --git a/ucb/source/ucp/file/filtask.hxx b/ucb/source/ucp/file/filtask.hxx index 2a427c6adaa6..24e719b3ebb8 100644 --- a/ucb/source/ucp/file/filtask.hxx +++ b/ucb/source/ucp/file/filtask.hxx @@ -585,12 +585,13 @@ namespace fileaccess // Special optimized method for getting the properties of a directoryitem, which // is returned by osl::DirectoryItem::getNextItem() - css::uno::Reference< css::sdbc::XRow > SAL_CALL + bool SAL_CALL getv( Notifier* pNotifier, const css::uno::Sequence< css::beans::Property >& properties, osl::DirectoryItem& DirItem, OUString& aUnqPath, - bool& bIsRegular ); + bool& bIsRegular, + css::uno::Reference< css::sdbc::XRow > & row ); /** _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits