Skip to content

Commit 6d1b21e

Browse files
lgritzlgritz
committed
Revision: unique counter in objects with per-thread error state
It seems too troublesome to index per-thread error state by object address, since those objects may be deleted and the same address reused later for a different object of the same type. Instead, using a unique ID (initialized by an incrementing atomic counter) to index the object in the per-thread error store. Signed-off-by: Larry Gritz <lg@larrygritz.com>
1 parent 76cb77d commit 6d1b21e

File tree

5 files changed

+43
-53
lines changed

5 files changed

+43
-53
lines changed

‎src/libOpenImageIO/imageinput.cpp‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,20 @@ using namespace pvt;
2727

2828

2929
// store an error message per thread, for a specific ImageInput
30-
static thread_local tsl::robin_map<const ImageInput*, std::string>
31-
input_error_messages;
30+
static thread_local tsl::robin_map<uint64_t, std::string> input_error_messages;
31+
static std::atomic_int64_t input_next_id(0);
32+
3233

3334
class ImageInput::Impl {
3435
public:
36+
Impl()
37+
: m_id(++input_next_id)
38+
{
39+
}
40+
3541
// So we can lock this ImageInput for the thread-safe methods.
3642
std::recursive_mutex m_mutex;
43+
uint64_t m_id;
3744
int m_threads = 0;
3845

3946
// The IOProxy object we will use for all I/O operations.
@@ -77,13 +84,6 @@ ImageInput::operator delete(void* ptr)
7784
ImageInput::ImageInput()
7885
: m_impl(new Impl, impl_deleter)
7986
{
80-
// Ensure that if this ImageInput has the same address as a previous one
81-
// that has been destroyed, we don't somehow inherit its error state!
82-
auto iter = input_error_messages.find(this);
83-
if (iter != input_error_messages.end()) {
84-
// print("Recycled ImageInput had error state\n");
85-
input_error_messages.erase(iter);
86-
}
8787
}
8888

8989

@@ -93,7 +93,7 @@ ImageInput::~ImageInput()
9393
// Erase any leftover errors from this thread
9494
// TODO: can we clear other threads' errors?
9595
// TODO: potentially unsafe due to the static destruction order fiasco
96-
// input_error_messages.erase(this);
96+
// input_error_messages.erase(m_impl->m_id);
9797
}
9898

9999

@@ -1103,7 +1103,7 @@ ImageInput::append_error(string_view message) const
11031103
{
11041104
if (message.size() && message.back() == '\n')
11051105
message.remove_suffix(1);
1106-
std::string& err_str = input_error_messages[this];
1106+
std::string& err_str = input_error_messages[m_impl->m_id];
11071107
OIIO_DASSERT(
11081108
err_str.size() < 1024 * 1024 * 16
11091109
&& "Accumulated error messages > 16MB. Try checking return codes!");
@@ -1119,7 +1119,7 @@ ImageInput::append_error(string_view message) const
11191119
bool
11201120
ImageInput::has_error() const
11211121
{
1122-
auto iter = input_error_messages.find(this);
1122+
auto iter = input_error_messages.find(m_impl->m_id);
11231123
if (iter == input_error_messages.end())
11241124
return false;
11251125
return iter.value().size() > 0;
@@ -1131,7 +1131,7 @@ std::string
11311131
ImageInput::geterror(bool clear) const
11321132
{
11331133
std::string e;
1134-
auto iter = input_error_messages.find(this);
1134+
auto iter = input_error_messages.find(m_impl->m_id);
11351135
if (iter != input_error_messages.end()) {
11361136
e = iter.value();
11371137
if (clear)

‎src/libOpenImageIO/imageoutput.cpp‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,22 @@ using namespace pvt;
3030

3131

3232
// store an error message per thread, for a specific ImageInput
33-
static thread_local tsl::robin_map<const ImageOutput*, std::string>
34-
output_error_messages;
33+
static thread_local tsl::robin_map<uint64_t, std::string> output_error_messages;
34+
static std::atomic_int64_t output_next_id(0);
3535

3636

3737
class ImageOutput::Impl {
3838
public:
39+
Impl()
40+
: m_id(++output_next_id)
41+
{
42+
}
43+
3944
// Unneeded?
4045
// // So we can lock this ImageOutput for the thread-safe methods.
4146
// std::recursive_mutex m_mutex;
4247

48+
uint64_t m_id;
4349
int m_threads = 0;
4450

4551
// The IOProxy object we will use for all I/O operations.
@@ -82,7 +88,7 @@ ImageOutput::ImageOutput()
8288
{
8389
// Ensure that if this ImageOutput has the same address as a previous one
8490
// that has been destroyed, we don't somehow inherit its error state!
85-
auto iter = output_error_messages.find(this);
91+
auto iter = output_error_messages.find(m_impl->m_id);
8692
if (iter != output_error_messages.end()) {
8793
// print("Recycled ImageOutput had error state\n");
8894
output_error_messages.erase(iter);
@@ -276,7 +282,7 @@ ImageOutput::append_error(string_view message) const
276282
{
277283
if (message.size() && message.back() == '\n')
278284
message.remove_suffix(1);
279-
std::string& err_str = output_error_messages[this];
285+
std::string& err_str = output_error_messages[m_impl->m_id];
280286
OIIO_DASSERT(
281287
err_str.size() < 1024 * 1024 * 16
282288
&& "Accumulated error messages > 16MB. Try checking return codes!");
@@ -705,7 +711,7 @@ ImageOutput::copy_tile_to_image_buffer(int x, int y, int z, TypeDesc format,
705711
bool
706712
ImageOutput::has_error() const
707713
{
708-
auto iter = output_error_messages.find(this);
714+
auto iter = output_error_messages.find(m_impl->m_id);
709715
if (iter == output_error_messages.end())
710716
return false;
711717
return iter.value().size() > 0;
@@ -717,7 +723,7 @@ std::string
717723
ImageOutput::geterror(bool clear) const
718724
{
719725
std::string e;
720-
auto iter = output_error_messages.find(this);
726+
auto iter = output_error_messages.find(m_impl->m_id);
721727
if (iter != output_error_messages.end()) {
722728
e = iter.value();
723729
if (clear)

‎src/libtexture/imagecache.cpp‎

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,7 @@ spin_mutex ImageCacheImpl::m_perthread_info_mutex;
4949

5050
namespace { // anonymous
5151

52-
static thread_local tsl::robin_map<const ImageCacheImpl*, std::string>
53-
imcache_error_messages;
54-
52+
static thread_local tsl::robin_map<uint64_t, std::string> imcache_error_messages;
5553
static std::shared_ptr<ImageCacheImpl> shared_image_cache;
5654
static spin_mutex shared_image_cache_mutex;
5755

@@ -1680,14 +1678,6 @@ ImageCacheImpl::set_max_open_files(int max_open_files)
16801678
void
16811679
ImageCacheImpl::init()
16821680
{
1683-
// Ensure that if this IC has the same address as a previous one
1684-
// that has been destroyed, we don't somehow inherit its error state!
1685-
auto iter = imcache_error_messages.find(this);
1686-
if (iter != imcache_error_messages.end()) {
1687-
// print("Recycled ImageCache had error state\n");
1688-
imcache_error_messages.erase(iter);
1689-
}
1690-
16911681
set_max_open_files(100);
16921682
m_max_memory_bytes = 1024LL * 1024 * 1024; // 1 GB default cache size
16931683
m_autotile = 0;
@@ -1733,7 +1723,7 @@ ImageCacheImpl::~ImageCacheImpl()
17331723
// Erase any leftover errors from this thread
17341724
// TODO: can we clear other threads' errors?
17351725
// TODO: potentially unsafe due to the static destruction order fiasco
1736-
// imcache_error_messages.erase(this);
1726+
// imcache_error_messages.erase(imagecache_id_atomic);
17371727
}
17381728

17391729

@@ -2743,7 +2733,7 @@ ImageCacheImpl::get_image_info(ImageCacheFile* file,
27432733
// for the file to be nonexistent or broken!
27442734
if ((*(int*)data = (file && !file->broken())) == 0) {
27452735
// eat any error generated by find_file
2746-
imcache_error_messages.erase(this);
2736+
imcache_error_messages.erase(imagecache_id_atomic);
27472737
(void)OIIO::geterror(true); // Suppress global errors
27482738
}
27492739
return true;
@@ -2771,7 +2761,7 @@ ImageCacheImpl::get_image_info(ImageCacheFile* file,
27712761
if (file->broken()) {
27722762
if (file->errors_should_issue()) {
27732763
// eat any earlier messages
2774-
imcache_error_messages.erase(this);
2764+
imcache_error_messages.erase(imagecache_id_atomic);
27752765
error("Invalid image file \"{}\": {}", file->filename(),
27762766
file->broken_error_message());
27772767
}
@@ -3886,7 +3876,7 @@ ImageCacheImpl::purge_perthread_microcaches()
38863876
bool
38873877
ImageCacheImpl::has_error() const
38883878
{
3889-
auto iter = imcache_error_messages.find(this);
3879+
auto iter = imcache_error_messages.find(imagecache_id_atomic);
38903880
if (iter == imcache_error_messages.end())
38913881
return false;
38923882
return iter.value().size() > 0;
@@ -3898,7 +3888,7 @@ std::string
38983888
ImageCacheImpl::geterror(bool clear) const
38993889
{
39003890
std::string e;
3901-
auto iter = imcache_error_messages.find(this);
3891+
auto iter = imcache_error_messages.find(imagecache_id_atomic);
39023892
if (iter != imcache_error_messages.end()) {
39033893
e = iter.value();
39043894
if (clear)
@@ -3914,7 +3904,7 @@ ImageCacheImpl::append_error(string_view message) const
39143904
{
39153905
if (message.size() && message.back() == '\n')
39163906
message.remove_suffix(1);
3917-
std::string& err_str = imcache_error_messages[this];
3907+
std::string& err_str = imcache_error_messages[imagecache_id_atomic];
39183908
OIIO_DASSERT(
39193909
err_str.size() < 1024 * 1024 * 16
39203910
&& "Accumulated error messages > 16MB. Try checking return codes!");

‎src/libtexture/texture_pvt.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -551,9 +551,10 @@ class TextureSystemImpl final : public TextureSystem {
551551
float dsdy, float dtdy, float sblur, float tblur);
552552

553553
ImageCacheImpl* m_imagecache = nullptr;
554-
bool m_imagecache_owner = false; ///< True if we own the ImageCache
555-
Imath::M44f m_Mw2c; ///< world-to-"common" matrix
556-
Imath::M44f m_Mc2w; ///< common-to-world matrix
554+
uint64_t m_id; // A unique ID for this TextureSystem
555+
Imath::M44f m_Mw2c; ///< world-to-"common" matrix
556+
Imath::M44f m_Mc2w; ///< common-to-world matrix
557+
bool m_imagecache_owner = false; ///< True if we own the ImageCache
557558
bool m_gray_to_rgb; ///< automatically copy gray to rgb channels?
558559
bool m_flip_t; ///< Flip direction of t coord?
559560
int m_max_tile_channels; ///< narrow tile ID channel range when

‎src/libtexture/texturesys.cpp‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ static spin_mutex shared_texturesys_mutex;
5353
static bool do_unit_test_texture = false;
5454
static float unit_test_texture_blur = 0.0f;
5555

56-
static thread_local tsl::robin_map<const TextureSystemImpl*, std::string>
57-
txsys_error_messages;
56+
static thread_local tsl::robin_map<int64_t, std::string> txsys_error_messages;
57+
static std::atomic_int64_t txsys_next_id(0);
5858

5959
static vfloat4 u8scale(1.0f / 255.0f);
6060
static vfloat4 u16scale(1.0f / 65535.0f);
@@ -326,6 +326,7 @@ texture_type_name(TexFormat f)
326326

327327

328328
TextureSystemImpl::TextureSystemImpl(ImageCache* imagecache)
329+
: m_id(++txsys_next_id)
329330
{
330331
m_imagecache = (ImageCacheImpl*)imagecache;
331332
init();
@@ -336,14 +337,6 @@ TextureSystemImpl::TextureSystemImpl(ImageCache* imagecache)
336337
void
337338
TextureSystemImpl::init()
338339
{
339-
// Ensure that if this TS has the same address as a previous one
340-
// that has been destroyed, we don't somehow inherit its error state!
341-
auto iter = txsys_error_messages.find(this);
342-
if (iter != txsys_error_messages.end()) {
343-
// print("Recycled TextureSystem had error state\n");
344-
txsys_error_messages.erase(iter);
345-
}
346-
347340
m_Mw2c.makeIdentity();
348341
m_gray_to_rgb = false;
349342
m_flip_t = false;
@@ -369,7 +362,7 @@ TextureSystemImpl::~TextureSystemImpl()
369362
// Erase any leftover errors from this thread
370363
// TODO: can we clear other threads' errors?
371364
// TODO: potentially unsafe due to the static destruction order fiasco
372-
// txsys_error_messages.erase(this);
365+
// txsys_error_messages.erase(m_id);
373366
}
374367

375368

@@ -916,7 +909,7 @@ TextureSystemImpl::get_texels(TextureHandle* texture_handle_,
916909
bool
917910
TextureSystemImpl::has_error() const
918911
{
919-
auto iter = txsys_error_messages.find(this);
912+
auto iter = txsys_error_messages.find(m_id);
920913
if (iter == txsys_error_messages.end())
921914
return false;
922915
return iter.value().size() > 0;
@@ -928,7 +921,7 @@ std::string
928921
TextureSystemImpl::geterror(bool clear) const
929922
{
930923
std::string e;
931-
auto iter = txsys_error_messages.find(this);
924+
auto iter = txsys_error_messages.find(m_id);
932925
if (iter != txsys_error_messages.end()) {
933926
e = iter.value();
934927
if (clear)
@@ -944,7 +937,7 @@ TextureSystemImpl::append_error(string_view message) const
944937
{
945938
if (message.size() && message.back() == '\n')
946939
message.remove_suffix(1);
947-
std::string& err_str = txsys_error_messages[this];
940+
std::string& err_str = txsys_error_messages[m_id];
948941
OIIO_DASSERT(
949942
err_str.size() < 1024 * 1024 * 16
950943
&& "Accumulated error messages > 16MB. Try checking return codes!");

0 commit comments

Comments
 (0)