Why does AudioThumbnail::setDataSource need the message manager locked?

General JUCE discussion
1

tl;dr: Why does AudioThumbnail::setDataSource require the message manager to be locked?

I just tripped across this assert while attempting to load/create thumbnails for my tracks in a background thread:

bool AudioThumbnail::setDataSource (LevelDataSource* newSource)
{
    JUCE_ASSERT_MESSAGE_MANAGER_IS_LOCKED

None of the code in that method does anything with the message manager, so why is it requiring it to be locked?

To attempt to answer that myself I dug through the history of that one line:

I know 9 years is a long time ago - I don’t remember why I coded what 1 year ago!

As a test I commented out the macro and didn’t experience any additional ill effects in my application.

If it is not needed could it be removed so that thumbnail creation can be done easily outside of the message loop without having to do an additional lock on it? If it is needed, then a code comment explaining why would help posterity. I can submit a PR if wanted once it’s determined either way.

2

Added a PR at https://github.com/WeAreROLI/JUCE/pull/542

3

Previously that assertion would flag up some cases where you’d be risking a data race. For example, if the AudioThumbnail was currently drawing to the screen on the message thread (in AudioThumbnail::drawChannel), then calling setDataSource from a different thread will cause trouble.

Adding more locks to make this safer is not trivial.

4

That risk and the non-trivial general solution are quite understandable. Clarification on “previously”, does that mean that the data race is no longer an issue, or because of my PR? I ask because I could change the PR to have a bypassable assert that has a comment explaining the issues. By “bypassable” I mean #ifndef or something better if there’s an an existing JUCE technique for this.

5

Sorry for the lack of clarity - “previously” in that context meant “before your PR”.

Adding some preprocessor code to get around the assertion feels a little heavy handed. A better approach would be to work out exactly what needs locking to make this thread safe, but it will need a bit of thought to get that right.