Assorted threading and undefined behaviour fixes

modules/juce_analytics/destinations/juce_ThreadedAnalyticsDestination.cpp

@@ -176,7 +176,7 @@ namespace DestinationTestHelpers
                           std::deque<AnalyticsEvent>& unloggedEvents)
             : TestDestination (loggedEvents, unloggedEvents)
         {
-            startAnalyticsThread (100);
+            startAnalyticsThread (20);
         }
 
         virtual ~BasicDestination()
@@ -303,12 +303,14 @@ struct ThreadedAnalyticsDestinationTests   : public UnitTest
 
         beginTest ("Basic");
         {
-            DestinationTestHelpers::BasicDestination destination (loggedEvents, unloggedEvents);
+            {
+                DestinationTestHelpers::BasicDestination destination (loggedEvents, unloggedEvents);
 
-            for (auto& event : testEvents)
-                destination.logEvent (event);
+                for (auto& event : testEvents)
+                    destination.logEvent (event);
 
-            Thread::sleep (400);
+                Thread::sleep (400);
+            }
 
             compareEventQueues (loggedEvents, testEvents);
             expect (unloggedEvents.size() == 0);

modules/juce_audio_devices/audio_io/juce_AudioDeviceManager.cpp

@@ -932,6 +932,8 @@ AudioDeviceManager::LevelMeter::LevelMeter() noexcept : level() {}
 
 void AudioDeviceManager::LevelMeter::updateLevel (const float* const* channelData, int numChannels, int numSamples) noexcept
 {
+    auto localLevel = level.get();
+
     if (enabled.get() != 0 && numChannels > 0)
     {
         for (int j = 0; j < numSamples; ++j)
@@ -943,20 +945,22 @@ void AudioDeviceManager::LevelMeter::updateLevel (const float* const* channelDat
 
             s /= (float) numChannels;
 
-            const double decayFactor = 0.99992;
+            const float decayFactor = 0.99992f;
 
-            if (s > level)
-                level = s;
-            else if (level > 0.001f)
-                level *= decayFactor;
+            if (s > localLevel)
+                localLevel = s;
+            else if (localLevel > 0.001f)
+                localLevel *= decayFactor;
             else
-                level = 0;
+                localLevel = 0;
         }
     }
     else
     {
-        level = 0;
+        localLevel = 0;
     }
+
+    level = localLevel;
 }
 
 void AudioDeviceManager::LevelMeter::setEnabled (bool shouldBeEnabled) noexcept
@@ -968,7 +972,7 @@ void AudioDeviceManager::LevelMeter::setEnabled (bool shouldBeEnabled) noexcept
 double AudioDeviceManager::LevelMeter::getCurrentLevel() const noexcept
 {
     jassert (enabled.get() != 0); // you need to call setEnabled (true) before using this!
-    return level;
+    return level.get();
 }
 
 void AudioDeviceManager::playTestSound()

modules/juce_audio_devices/audio_io/juce_AudioDeviceManager.h

@@ -488,7 +488,7 @@ private:
         double getCurrentLevel() const noexcept;
 
         Atomic<int> enabled;
-        double level;
+        Atomic<float> level;
     };
 
     LevelMeter inputLevelMeter, outputLevelMeter;

modules/juce_core/threads/juce_SpinLock.h

@@ -62,7 +62,7 @@ public:
     /** Releases the lock. */
     inline void exit() const noexcept
     {
-        jassert (lock.value == 1); // Agh! Releasing a lock that isn't currently held!
+        jassert (lock.get() == 1); // Agh! Releasing a lock that isn't currently held!
         lock = 0;
     }
 

modules/juce_core/zip/juce_ZipFile.cpp

@@ -23,17 +23,30 @@
 namespace juce
 {
 
+uint16 readUnalignedLittleEndianShort (const void* buffer)
+{
+    auto data = readUnaligned<uint16> (buffer);
+    return ByteOrder::littleEndianShort (&data);
+}
+
+uint32 readUnalignedLittleEndianInt (const void* buffer)
+{
+    auto data = readUnaligned<uint32> (buffer);
+    return ByteOrder::littleEndianInt (&data);
+}
+
 struct ZipFile::ZipEntryHolder
 {
     ZipEntryHolder (const char* buffer, int fileNameLen)
     {
-        isCompressed            = ByteOrder::littleEndianShort (buffer + 10) != 0;
-        entry.fileTime          = parseFileTime (ByteOrder::littleEndianShort (buffer + 12),
-                                                 ByteOrder::littleEndianShort (buffer + 14));
-        compressedSize          = (int64) ByteOrder::littleEndianInt (buffer + 20);
-        entry.uncompressedSize  = (int64) ByteOrder::littleEndianInt (buffer + 24);
-        streamOffset            = (int64) ByteOrder::littleEndianInt (buffer + 42);
-        entry.filename          = String::fromUTF8 (buffer + 46, fileNameLen);
+        isCompressed           = readUnalignedLittleEndianShort (buffer + 10) != 0;
+        entry.fileTime         = parseFileTime (readUnalignedLittleEndianShort (buffer + 12),
+                                                readUnalignedLittleEndianShort (buffer + 14));
+        compressedSize         = (int64) readUnalignedLittleEndianInt (buffer + 20);
+        entry.uncompressedSize = (int64) readUnalignedLittleEndianInt (buffer + 24);
+        streamOffset           = (int64) readUnalignedLittleEndianInt (buffer + 42);
+
+        entry.filename = String::fromUTF8 (buffer + 46, fileNameLen);
     }
 
     static Time parseFileTime (uint32 time, uint32 date) noexcept
@@ -74,12 +87,12 @@ static int64 findCentralDirectoryFileHeader (InputStream& input, int& numEntries
 
         for (int i = 0; i < 22; ++i)
         {
-            if (ByteOrder::littleEndianInt (buffer + i) == 0x06054b50)
+            if (readUnalignedLittleEndianInt (buffer + i) == 0x06054b50)
             {
                 in.setPosition (pos + i);
                 in.read (buffer, 22);
-                numEntries = ByteOrder::littleEndianShort (buffer + 10);
-                auto offset = (int64) ByteOrder::littleEndianInt (buffer + 16);
+                numEntries = readUnalignedLittleEndianShort (buffer + 10);
+                auto offset = (int64) readUnalignedLittleEndianInt (buffer + 16);
 
                 if (offset >= 4)
                 {
@@ -351,7 +364,7 @@ void ZipFile::init()
                         break;
 
                     auto* buffer = static_cast<const char*> (headerData.getData()) + pos;
-                    auto fileNameLen = ByteOrder::littleEndianShort (buffer + 28);
+                    auto fileNameLen = readUnalignedLittleEndianShort (buffer + 28);
 
                     if (pos + 46 + fileNameLen > size)
                         break;
@@ -359,8 +372,8 @@ void ZipFile::init()
                     entries.add (new ZipEntryHolder (buffer, fileNameLen));
 
                     pos += 46 + fileNameLen
-                            + ByteOrder::littleEndianShort (buffer + 30)
-                            + ByteOrder::littleEndianShort (buffer + 32);
+                            + readUnalignedLittleEndianShort (buffer + 30)
+                            + readUnalignedLittleEndianShort (buffer + 32);
                 }
             }
         }
@@ -600,4 +613,50 @@ bool ZipFile::Builder::writeToStream (OutputStream& target, double* const progre
     return true;
 }
 
+//==============================================================================
+#if JUCE_UNIT_TESTS
+
+struct ZIPTests   : public UnitTest
+{
+    ZIPTests()   : UnitTest ("ZIP") {}
+
+    void runTest() override
+    {
+        beginTest ("ZIP");
+
+        ZipFile::Builder builder;
+        StringArray entryNames { "first", "second", "third" };
+        HashMap<String, MemoryBlock> blocks;
+
+        for (auto& entryName : entryNames)
+        {
+            auto& block = blocks.getReference (entryName);
+            MemoryOutputStream mo (block, false);
+            mo << entryName;
+            mo.flush();
+            builder.addEntry (new MemoryInputStream (block, false), 9, entryName, Time::getCurrentTime());
+        }
+
+        MemoryBlock data;
+        MemoryOutputStream mo (data, false);
+        builder.writeToStream (mo, nullptr);
+        MemoryInputStream mi (data, false);
+
+        ZipFile zip (mi);
+
+        expectEquals (zip.getNumEntries(), entryNames.size());
+
+        for (auto& entryName : entryNames)
+        {
+            auto* entry = zip.getEntry (entryName);
+            ScopedPointer<InputStream> input (zip.createStreamForEntry (*entry));
+            expectEquals (input->readEntireStreamAsString(), entryName);
+        }
+    }
+};
+
+static ZIPTests zipTests;
+
+#endif
+
 } // namespace juce

modules/juce_events/messages/juce_MessageManager.cpp

@@ -68,7 +68,7 @@ bool MessageManager::MessageBase::post()
 {
     auto* mm = MessageManager::instance;
 
-    if (mm == nullptr || mm->quitMessagePosted || ! postMessageToSystemQueue (this))
+    if (mm == nullptr || mm->quitMessagePosted.get() != 0 || ! postMessageToSystemQueue (this))
     {
         Ptr deleter (this); // (this will delete messages that were just created with a 0 ref count)
         return false;
@@ -85,7 +85,7 @@ bool MessageManager::runDispatchLoopUntil (int millisecondsToRunFor)
 
     auto endTime = Time::currentTimeMillis() + millisecondsToRunFor;
 
-    while (! quitMessageReceived)
+    while (quitMessageReceived.get() == 0)
     {
         JUCE_TRY
         {
@@ -98,7 +98,7 @@ bool MessageManager::runDispatchLoopUntil (int millisecondsToRunFor)
             break;
     }
 
-    return ! quitMessageReceived;
+    return quitMessageReceived.get() == 0;
 }
 #endif
 
@@ -121,7 +121,7 @@ void MessageManager::runDispatchLoop()
 {
     jassert (isThisTheMessageThread()); // must only be called by the message thread
 
-    while (! quitMessageReceived)
+    while (quitMessageReceived.get() == 0)
     {
         JUCE_TRY
         {

modules/juce_events/messages/juce_MessageManager.h

@@ -80,7 +80,7 @@ public:
 
     /** Returns true if the stopDispatchLoop() method has been called.
     */
-    bool hasStopMessageBeenSent() const noexcept        { return quitMessagePosted; }
+    bool hasStopMessageBeenSent() const noexcept        { return quitMessagePosted.get() != 0; }
 
    #if JUCE_MODAL_LOOPS_PERMITTED || DOXYGEN
     /** Synchronously dispatches messages until a given time has elapsed.
@@ -318,7 +318,7 @@ private:
     friend class MessageManagerLock;
 
     ScopedPointer<ActionBroadcaster> broadcaster;
-    bool quitMessagePosted = false, quitMessageReceived = false;
+    Atomic<int> quitMessagePosted { 0 }, quitMessageReceived { 0 };
     Thread::ThreadID messageThreadId;
     Atomic<Thread::ThreadID> threadWithLock;
 

modules/juce_events/native/juce_ios_MessageManager.mm

@@ -27,7 +27,7 @@ void MessageManager::runDispatchLoop()
 {
     jassert (isThisTheMessageThread()); // must only be called by the message thread
 
-    while (! quitMessagePosted)
+    while (quitMessagePosted.get() == 0)
     {
         JUCE_AUTORELEASEPOOL
         {
@@ -55,7 +55,7 @@ bool MessageManager::runDispatchLoopUntil (int millisecondsToRunFor)
         uint32 startTime = Time::getMillisecondCounter();
         NSDate* endDate = [NSDate dateWithTimeIntervalSinceNow: millisecondsToRunFor * 0.001];
 
-        while (! quitMessagePosted)
+        while (quitMessagePosted.get() == 0)
         {
             JUCE_AUTORELEASEPOOL
             {
@@ -68,7 +68,7 @@ bool MessageManager::runDispatchLoopUntil (int millisecondsToRunFor)
             }
         }
 
-        return ! quitMessagePosted;
+        return quitMessagePosted.get() == 0;
     }
 }
 #endif

modules/juce_events/native/juce_mac_MessageManager.mm

@@ -313,7 +313,7 @@ private:
 //==============================================================================
 void MessageManager::runDispatchLoop()
 {
-    if (! quitMessagePosted) // check that the quit message wasn't already posted..
+    if (quitMessagePosted.get() == 0) // check that the quit message wasn't already posted..
     {
         JUCE_AUTORELEASEPOOL
         {
@@ -383,7 +383,7 @@ bool MessageManager::runDispatchLoopUntil (int millisecondsToRunFor)
 
     uint32 endTime = Time::getMillisecondCounter() + (uint32) millisecondsToRunFor;
 
-    while (! quitMessagePosted)
+    while (quitMessagePosted.get() == 0)
     {
         JUCE_AUTORELEASEPOOL
         {
@@ -402,7 +402,7 @@ bool MessageManager::runDispatchLoopUntil (int millisecondsToRunFor)
         }
     }
 
-    return ! quitMessagePosted;
+    return quitMessagePosted.get() == 0;
 }
 #endif
 

modules/juce_gui_basics/filebrowser/juce_DirectoryContentsList.cpp

@@ -120,6 +120,13 @@ void DirectoryContentsList::setFileFilter (const FileFilter* newFileFilter)
 }
 
 //==============================================================================
+int DirectoryContentsList::getNumFiles() const noexcept
+{
+    const ScopedLock sl (fileListLock);
+
+    return files.size();
+}
+
 bool DirectoryContentsList::getFileInfo (const int index, FileInfo& result) const
 {
     const ScopedLock sl (fileListLock);

modules/juce_gui_basics/filebrowser/juce_DirectoryContentsList.h

@@ -163,7 +163,7 @@ public:
 
         @see getFileInfo, getFile
     */
-    int getNumFiles() const noexcept                        { return files.size(); }
+    int getNumFiles() const noexcept;
 
     /** Returns the cached information about one of the files in the list.
 

modules/juce_gui_basics/filebrowser/juce_FileTreeComponent.cpp

@@ -177,6 +177,8 @@ public:
 
     void paintItem (Graphics& g, int width, int height) override
     {
+        ScopedLock lock (iconUpdate);
+
         if (file != File())
         {
             updateIcon (true);
@@ -229,6 +231,7 @@ private:
     OptionalScopedPointer<DirectoryContentsList> subContentsList;
     bool isDirectory;
     TimeSliceThread& thread;
+    CriticalSection iconUpdate;
     Image icon;
     String fileSize, modTime;
 
@@ -249,7 +252,11 @@ private:
 
             if (im.isValid())
             {
-                icon = im;
+                {
+                    ScopedLock lock (iconUpdate);
+                    icon = im;
+                }
+
                 triggerAsyncUpdate();
             }
         }