mir/libreoffice
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

tsan: fix data race in o3tl::cow_wrapper

Browse Source 
where the use_count() call was happening without any kind of
synchronisation - switch to use std::atomic which does the right thing
for us

Change-Id: I79a6452f42bd425ea494bb0298dc134de5678813
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170217
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
This commit is contained in:
Noel Grandin
2024-07-09 10:23:27 +02:00
parent 02786af7ea
commit c04de7a6f2
3 changed files with 15 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
include/o3tl/cow_wrapper.hxx
View File

@@ -20,8 +20,7 @@
#ifndef INCLUDED_O3TL_COW_WRAPPER_HXX #ifndef INCLUDED_O3TL_COW_WRAPPER_HXX
#define INCLUDED_O3TL_COW_WRAPPER_HXX #define INCLUDED_O3TL_COW_WRAPPER_HXX
#include <osl/interlck.h> #include <atomic>
#include <optional> #include <optional>
#include <cstddef> #include <cstddef>
@@ -38,6 +37,7 @@ namespace o3tl
typedef std::size_t ref_count_t; typedef std::size_t ref_count_t;
static void incrementCount( ref_count_t& rCount ) { ++rCount; } static void incrementCount( ref_count_t& rCount ) { ++rCount; }
static bool decrementCount( ref_count_t& rCount ) { return --rCount != 0; } static bool decrementCount( ref_count_t& rCount ) { return --rCount != 0; }
static std::size_t getCount( ref_count_t& rCount) { return rCount; }
}; };
/** Thread-safe refcounting /** Thread-safe refcounting
@@ -47,12 +47,13 @@ namespace o3tl
*/ */
struct ThreadSafeRefCountingPolicy struct ThreadSafeRefCountingPolicy
{ {
typedef oslInterlockedCount ref_count_t; typedef std::atomic<int> ref_count_t;
static void incrementCount( ref_count_t& rCount ) { osl_atomic_increment(&rCount); } static void incrementCount( ref_count_t& rCount ) { rCount++; }
static bool decrementCount( ref_count_t& rCount ) static bool decrementCount( ref_count_t& rCount )
{ {
return osl_atomic_decrement(&rCount) != 0; return (--rCount) != 0;
} }
static std::size_t getCount( ref_count_t& rCount) { return rCount; }
}; };
/** Copy-on-write wrapper. /** Copy-on-write wrapper.
@@ -315,9 +316,9 @@ int cow_wrapper_client::queryUnmodified() const
} }
/// return number of shared instances (1 for unique object) /// return number of shared instances (1 for unique object)
typename MTPolicy::ref_count_t use_count() const // nothrow size_t use_count() const // nothrow
{ {
return m_pimpl ? m_pimpl->m_ref_count : 0; return m_pimpl ? MTPolicy::getCount(m_pimpl->m_ref_count) : 0;
} }
void swap(cow_wrapper& r) // never throws void swap(cow_wrapper& r) // never throws

4
o3tl/qa/cow_wrapper_clients.cxx
View File

@@ -93,7 +93,7 @@ bool cow_wrapper_client2::is_unique() const
{ {
return maImpl.is_unique(); return maImpl.is_unique();
} }
oslInterlockedCount cow_wrapper_client2::use_count() const int cow_wrapper_client2::use_count() const
{ {
return maImpl.use_count(); return maImpl.use_count();
} }
@@ -171,7 +171,7 @@ bool cow_wrapper_client3::is_unique() const
{ {
return maImpl.is_unique(); return maImpl.is_unique();
} }
oslInterlockedCount cow_wrapper_client3::use_count() const int cow_wrapper_client3::use_count() const
{ {
return maImpl.use_count(); return maImpl.use_count();
} }

8
o3tl/qa/cow_wrapper_clients.hxx
View File

@@ -20,6 +20,7 @@
#ifndef INCLUDED_O3TL_QA_COW_WRAPPER_CLIENTS_HXX #ifndef INCLUDED_O3TL_QA_COW_WRAPPER_CLIENTS_HXX
#define INCLUDED_O3TL_QA_COW_WRAPPER_CLIENTS_HXX #define INCLUDED_O3TL_QA_COW_WRAPPER_CLIENTS_HXX
#include <sal/types.h>
#include <o3tl/cow_wrapper.hxx> #include <o3tl/cow_wrapper.hxx>
#include <assert.h> #include <assert.h>
#include <iostream> #include <iostream>
@@ -45,7 +46,7 @@ public:
void makeUnique() { maImpl.make_unique(); } void makeUnique() { maImpl.make_unique(); }
bool is_unique() const { return maImpl.is_unique(); } bool is_unique() const { return maImpl.is_unique(); }
oslInterlockedCount use_count() const { return maImpl.use_count(); } int use_count() const { return maImpl.use_count(); }
void swap( cow_wrapper_client1& r ) { o3tl::swap(maImpl, r.maImpl); } void swap( cow_wrapper_client1& r ) { o3tl::swap(maImpl, r.maImpl); }
bool operator==( const cow_wrapper_client1& rRHS ) const { return maImpl == rRHS.maImpl; } bool operator==( const cow_wrapper_client1& rRHS ) const { return maImpl == rRHS.maImpl; }
@@ -79,7 +80,7 @@ public:
void makeUnique(); void makeUnique();
bool is_unique() const; bool is_unique() const;
oslInterlockedCount use_count() const; int use_count() const;
void swap( cow_wrapper_client2& r ); void swap( cow_wrapper_client2& r );
bool operator==( const cow_wrapper_client2& rRHS ) const; bool operator==( const cow_wrapper_client2& rRHS ) const;
@@ -110,7 +111,7 @@ public:
void makeUnique(); void makeUnique();
bool is_unique() const; bool is_unique() const;
oslInterlockedCount use_count() const; int use_count() const;
void swap( cow_wrapper_client3& r ); void swap( cow_wrapper_client3& r );
bool operator==( const cow_wrapper_client3& rRHS ) const; bool operator==( const cow_wrapper_client3& rRHS ) const;
@@ -172,6 +173,7 @@ struct BogusRefCountPolicy
} }
return rCount != 0; return rCount != 0;
} }
static std::size_t getCount( ref_count_t& rCount) { return rCount; }
}; };
class cow_wrapper_client5 class cow_wrapper_client5