Browse Source

don't nullify on SafeRelease

That is generally not necessary and sometimes even leads to defects,
like access to just-deleted area
alexey 3 years ago
parent
commit
f13d4e217e
6 changed files with 13 additions and 12 deletions
  1. 1 0
      src/.clang-format
  2. 0 1
      src/gtests/gtests_searchdaemon.cpp
  3. 5 5
      src/searchdha.cpp
  4. 1 1
      src/sphinxquery.cpp
  5. 1 1
      src/sphinxsearch.cpp
  6. 5 4
      src/std/generics.h

+ 1 - 0
src/.clang-format

@@ -147,6 +147,7 @@ StatementMacros:
   - SafeDelete
   - SafeDeleteArray
   - SafeRelease
+  - SafeReleaseAndZero
   - SafeAddRef
   - sphWarning
   - sphInfo

+ 0 - 1
src/gtests/gtests_searchdaemon.cpp

@@ -581,7 +581,6 @@ protected:
 	{
 		SafeDelete ( pHash );
 		ASSERT_TRUE ( pRef->IsLast() ) << "hash deleted, we a the one";
-		SafeRelease ( pRef );
 	}
 };
 

+ 5 - 5
src/searchdha.cpp

@@ -2066,7 +2066,7 @@ void ScheduleDistrJobs ( VectorAgentConn_t &dRemotes, RequestBuilder_i * pQuery,
 		if ( pConnection->IsBlackhole () )
 		{
 			sphLogDebugv ( "S Remove blackhole()" );
-			SafeRelease ( pConnection );
+			SafeReleaseAndZero ( pConnection );
 			dRemotes.RemoveFast ( i-- );
 			pReporter->FeedTask ( false );
 		}
@@ -2093,15 +2093,15 @@ public:
 	void FeedTask ( bool bAdd ) final
 	{
 		if ( bAdd )
-			SafeAddRef ( m_pConnection )
+			SafeAddRef ( m_pConnection );
 		else
-			SafeRelease ( m_pConnection ) // yes, SafeRelease will make m_pConnection=0 at the end!
+			SafeReleaseAndZero ( m_pConnection ); // yes, SafeReleaseAndZero will make m_pConnection=0 at the end!
 	}
 
 	void Report ( bool bParam ) final
 	{
 		assert ( m_pConnection && "strange Report called without the connection!" );
-		SafeRelease ( m_pConnection )
+		SafeReleaseAndZero ( m_pConnection );
 		if ( m_pCallback )
 			m_pCallback ( bParam );
 	}
@@ -3234,7 +3234,7 @@ private:
 		SafeDelete ( pTask );
 
 		if ( bReleasePayload )
-			SafeRelease ( pConnection );
+			SafeReleaseAndZero ( pConnection );
 		return pConnection;
 	}
 

+ 1 - 1
src/sphinxquery.cpp

@@ -434,7 +434,7 @@ XQNode_t * XQParseHelper_c::SweepNulls ( XQNode_t * pNode )
 			ARRAY_FOREACH ( i, pRet->m_dChildren )
 			{
 				m_dSpawned.RemoveValue ( pRet->m_dChildren[i] );
-				SafeDelete ( pRet->m_dChildren[i] )
+				SafeDelete ( pRet->m_dChildren[i] );
 			}
 			pRet->m_dChildren.Reset();
 		}

+ 1 - 1
src/sphinxsearch.cpp

@@ -728,7 +728,7 @@ void ExtRanker_c::FinalizeCache ( const ISphSchema & tSorterSchema )
 		QcacheAdd ( m_pCtx->m_tQuery, m_pQcacheEntry, tSorterSchema );
 	}
 
-	SafeRelease ( m_pQcacheEntry );
+	SafeReleaseAndZero ( m_pQcacheEntry );
 }
 
 

+ 5 - 4
src/std/generics.h

@@ -45,10 +45,11 @@ typename std::common_type<T, U>::type Max ( T a, U b )
 {
 	return a < b ? b : a;
 }
-#define SafeDelete( _x )		{ if ( _x ) { delete ( _x ); ( _x ) = nullptr; } }
-#define SafeDeleteArray( _x )	{ if ( _x ) { delete[] ( _x ); ( _x ) = nullptr; } }
-#define SafeRelease( _x )		{ if ( _x ) { ( _x )->Release(); ( _x ) = nullptr; } }
-#define SafeAddRef( _x )        { if ( _x ) { ( _x )->AddRef(); } }
+#define SafeDelete( _x )		do { if ( _x ) { delete ( _x ); ( _x ) = nullptr; } } while ( 0 )
+#define SafeDeleteArray( _x )	do { if ( _x ) { delete[] ( _x ); ( _x ) = nullptr; } } while ( 0 )
+#define SafeReleaseAndZero( _x)	do { if ( _x ) { ( _x )->Release(); ( _x ) = nullptr; } } while ( 0 )
+#define SafeRelease( _x )		do { if ( _x ) { ( _x )->Release(); } } while ( 0 )
+#define SafeAddRef( _x )        do { if ( _x ) { ( _x )->AddRef(); } } while ( 0 )
 
 /// swap
 template<typename T>