Data Race in AudioParameterBool

General JUCE discussion
21

That won’t help.

Take my previous example and change it a bit so that it looks like this:

Thread::launch ([this]()
{
    Image image2 (Image::PixelFormat::ARGB, 12, 12, true);
    imageToRenderTo[0] = std::move (image2);
});

auto image3 = imageToRenderTo[0].rescaled (4, 4);

Even if the actual pointer swap is atomic, the rescaled call is not safe. Either the pointer will swap between two calls rescaled makes into the image data and you’ll get garbage out, or the image data will be deleted under its feet when the moved from imageToRenderTo goes out of scope and the reference count drops to zero.

2 Likes
22

hey @t0m since the move semantics functions of ReferenceCountedObjectPtr aren’t thread safe, but copying a ReferenceCountedObjectPtr is threadsafe, doesn’t it make sense to get rid of the move semantics for that class?

23

What about when you want to move it in a normal, thread-safe context?

24

there doesn’t seem to be a point to those move ctor/assign functions since the whole incIfNotNull() part of the ReferenceCountedObject being owned by the ReferenceCountedObjectPtr handles preventing allocations when you copy.

i.e.

void f(Image img) { }

void test()
{
    Image image(Image::PixelDataFormat::RGB, 30, 30, true);
    f( image );   
}

no need to std::move(image) in that call to f() because image isn’t used after f takes a copy, so the reference counting stuff would handle deletion when f() ends.

It just seems like an atomically-reference-counted object doesn’t need move semantics to me…

25

The move ops are an optimisation. Every invocation of the copy ops will cause an atomic inc/dec on the internal reference count, which could be a lot slower than the move ops which just swap two non-atomic pointers.