From 0c3500115c4fd86284a027fc32be704afcf77061 Mon Sep 17 00:00:00 2001 From: Stephan Bergmann Date: Fri, 1 Mar 2013 16:50:18 +0100 Subject: Related rhbz#915743: Do not call into DAVResourceAccess with mutex locked ...from webdav Content::getResourceType, as otherwise Content::abort would be blocked waiting for the mutex (in code that would call abort, which will be required to fix rhbz#915743 "thread deadlock/slow join in insert->hyperlink in impress"). This required to get the odd reference to enum return type of getResourceType straight. Also, propagate information about !shouldAccessNetworkAfterException from getResourceType out to getPropertyValues, to avoid further calls that would again block/fail. Change-Id: I8b9d43a61eb4078acb90079c4eb7aa98a59a8983 --- ucb/source/ucp/webdav-neon/webdavcontent.cxx | 142 ++++++++++++++------------- ucb/source/ucp/webdav-neon/webdavcontent.hxx | 7 +- 2 files changed, 80 insertions(+), 69 deletions(-) (limited to 'ucb') diff --git a/ucb/source/ucp/webdav-neon/webdavcontent.cxx b/ucb/source/ucp/webdav-neon/webdavcontent.cxx index 5c2aa05d593d..2901b843b44e 100644 --- a/ucb/source/ucp/webdav-neon/webdavcontent.cxx +++ b/ucb/source/ucp/webdav-neon/webdavcontent.cxx @@ -782,8 +782,8 @@ void SAL_CALL Content::addProperty( const rtl::OUString& Name, { try { - const ResourceType & rType = getResourceType( xEnv ); - switch ( rType ) + ResourceType eType = getResourceType( xEnv ); + switch ( eType ) { case UNKNOWN: case DAV: @@ -877,8 +877,8 @@ void SAL_CALL Content::removeProperty( const rtl::OUString& Name ) { try { - const ResourceType & rType = getResourceType( xEnv ); - switch ( rType ) + ResourceType eType = getResourceType( xEnv ); + switch ( eType ) { case UNKNOWN: case DAV: @@ -1175,11 +1175,11 @@ uno::Reference< sdbc::XRow > Content::getPropertyValues( ///////////////////////////////////////////////////////////////////// // First, identify whether resource is DAV or not - const ResourceType & rType = getResourceType( xEnv, xResAccess ); - bool bNetworkAccessAllowed = true; + ResourceType eType = getResourceType( + xEnv, xResAccess, &bNetworkAccessAllowed ); - if ( DAV == rType ) + if ( eType == DAV ) { // cache lookup... getResourceType may fill the props cache via // PROPFIND! @@ -1269,8 +1269,8 @@ uno::Reference< sdbc::XRow > Content::getPropertyValues( } catch ( DAVException const & e ) { - bNetworkAccessAllowed - = shouldAccessNetworkAfterException( e ); + bNetworkAccessAllowed = bNetworkAccessAllowed + && shouldAccessNetworkAfterException( e ); if ( !bNetworkAccessAllowed ) { @@ -1341,7 +1341,7 @@ uno::Reference< sdbc::XRow > Content::getPropertyValues( NeonUri aUri( xResAccess->getURL() ); aUnescapedTitle = aUri.GetPathBaseNameUnescaped(); - if ( rType == UNKNOWN ) + if ( eType == UNKNOWN ) { xProps.reset( new ContentProperties( aUnescapedTitle ) ); } @@ -1349,7 +1349,7 @@ uno::Reference< sdbc::XRow > Content::getPropertyValues( // For DAV resources we only know the Title, for non-DAV // resources we additionally know that it is a document. - if ( rType == DAV ) + if ( eType == DAV ) { //xProps.reset( // new ContentProperties( aUnescapedTitle ) ); @@ -3070,83 +3070,93 @@ Content::getBaseURI( const std::auto_ptr< DAVResourceAccess > & rResAccess ) } //========================================================================= -const Content::ResourceType & Content::getResourceType( +Content::ResourceType Content::getResourceType( const uno::Reference< ucb::XCommandEnvironment >& xEnv, - const std::auto_ptr< DAVResourceAccess > & rResAccess ) + const std::auto_ptr< DAVResourceAccess > & rResAccess, + bool * networkAccessAllowed) throw ( uno::Exception ) { - if ( m_eResourceType == UNKNOWN ) { - osl::Guard< osl::Mutex > aGuard( m_aMutex ); + osl::MutexGuard g(m_aMutex); + if (m_eResourceType != UNKNOWN) { + return m_eResourceType; + } + } - ResourceType eResourceType; - eResourceType = m_eResourceType; + ResourceType eResourceType; - const rtl::OUString & rURL = rResAccess->getURL(); - const rtl::OUString aScheme( - rURL.copy( 0, rURL.indexOf( ':' ) ).toAsciiLowerCase() ); + const rtl::OUString & rURL = rResAccess->getURL(); + const rtl::OUString aScheme( + rURL.copy( 0, rURL.indexOf( ':' ) ).toAsciiLowerCase() ); - if ( aScheme == FTP_URL_SCHEME ) + if ( aScheme == FTP_URL_SCHEME ) + { + eResourceType = FTP; + } + else + { + try { - eResourceType = FTP; + // Try to fetch some frequently used property value, e.g. those + // used when loading documents... along with identifying whether + // this is a DAV resource. + std::vector< DAVResource > resources; + std::vector< rtl::OUString > aPropNames; + uno::Sequence< beans::Property > aProperties( 5 ); + aProperties[ 0 ].Name = rtl::OUString("IsFolder"); + aProperties[ 1 ].Name = rtl::OUString("IsDocument"); + aProperties[ 2 ].Name = rtl::OUString("IsReadOnly"); + aProperties[ 3 ].Name = rtl::OUString("MediaType"); + aProperties[ 4 ].Name = DAVProperties::SUPPORTEDLOCK; + + ContentProperties::UCBNamesToDAVNames( aProperties, aPropNames ); + + rResAccess->PROPFIND( DAVZERO, aPropNames, resources, xEnv ); + + if ( resources.size() == 1 ) + { + osl::MutexGuard g(m_aMutex); + m_xCachedProps.reset( + new CachableContentProperties( resources[ 0 ] ) ); + m_xCachedProps->containsAllNames( + aProperties, m_aFailedPropNames ); + } + + eResourceType = DAV; } - else + catch ( DAVException const & e ) { - try - { - // Try to fetch some frequently used property value, e.g. those - // used when loading documents... along with identifying whether - // this is a DAV resource. - std::vector< DAVResource > resources; - std::vector< rtl::OUString > aPropNames; - uno::Sequence< beans::Property > aProperties( 5 ); - aProperties[ 0 ].Name - = rtl::OUString("IsFolder"); - aProperties[ 1 ].Name - = rtl::OUString("IsDocument"); - aProperties[ 2 ].Name - = rtl::OUString("IsReadOnly"); - aProperties[ 3 ].Name - = rtl::OUString("MediaType"); - aProperties[ 4 ].Name - = DAVProperties::SUPPORTEDLOCK; - - ContentProperties::UCBNamesToDAVNames( - aProperties, aPropNames ); - - rResAccess->PROPFIND( - DAVZERO, aPropNames, resources, xEnv ); - - if ( resources.size() == 1 ) - { - m_xCachedProps.reset( - new CachableContentProperties( resources[ 0 ] ) ); - m_xCachedProps->containsAllNames( - aProperties, m_aFailedPropNames ); - } + rResAccess->resetUri(); - eResourceType = DAV; + if ( e.getStatus() == SC_METHOD_NOT_ALLOWED ) + { + // Status SC_METHOD_NOT_ALLOWED is a safe indicator that the + // resource is NON_DAV + eResourceType = NON_DAV; } - catch ( DAVException const & e ) + else if (networkAccessAllowed != 0) { - rResAccess->resetUri(); - - if ( e.getStatus() == SC_METHOD_NOT_ALLOWED ) - { - // Status SC_METHOD_NOT_ALLOWED is a safe indicator that the - // resource is NON_DAV - eResourceType = NON_DAV; - } + *networkAccessAllowed = *networkAccessAllowed + && shouldAccessNetworkAfterException(e); } } + } + + osl::MutexGuard g(m_aMutex); + if (m_eResourceType == UNKNOWN) { m_eResourceType = eResourceType; + } else { + SAL_WARN_IF( + eResourceType != m_eResourceType, "ucb.ucp.webdav", + "different resource types for <" << rURL << ">: " + << +eResourceType << " vs. " << +m_eResourceType); } return m_eResourceType; } SAL_WNODEPRECATED_DECLARATIONS_POP //========================================================================= -const Content::ResourceType & Content::getResourceType( +Content::ResourceType Content::getResourceType( const uno::Reference< ucb::XCommandEnvironment >& xEnv ) throw ( uno::Exception ) { diff --git a/ucb/source/ucp/webdav-neon/webdavcontent.hxx b/ucb/source/ucp/webdav-neon/webdavcontent.hxx index 637ab7f22df5..83afb3f8f5c2 100644 --- a/ucb/source/ucp/webdav-neon/webdavcontent.hxx +++ b/ucb/source/ucp/webdav-neon/webdavcontent.hxx @@ -133,16 +133,17 @@ private: getBaseURI( const std::auto_ptr< DAVResourceAccess > & rResAccess ); SAL_WNODEPRECATED_DECLARATIONS_POP - const ResourceType & + ResourceType getResourceType( const ::com::sun::star::uno::Reference< ::com::sun::star::ucb::XCommandEnvironment >& xEnv ) throw ( ::com::sun::star::uno::Exception ); SAL_WNODEPRECATED_DECLARATIONS_PUSH - const ResourceType & + ResourceType getResourceType( const ::com::sun::star::uno::Reference< ::com::sun::star::ucb::XCommandEnvironment >& xEnv, - const std::auto_ptr< DAVResourceAccess > & rResAccess ) + const std::auto_ptr< DAVResourceAccess > & rResAccess, + bool * networkAccessAllowed = 0) throw ( ::com::sun::star::uno::Exception ); SAL_WNODEPRECATED_DECLARATIONS_POP -- cgit v1.2.3