Crash in http connection thread, OSX


#1

Hello all,

We need to (regularly) get a URL from our app, an for this purpose we had built a ThreadPoolJob:

    class UrlGetJob : public ThreadPoolJob
    {
    public:
        UrlGetJob(String url);
        ~UrlGetJob();
        virtual ThreadPoolJob::JobStatus runJob();

    private:
        String m_Url;
        static const int cTimeout = 1000;
    };

    UrlGetJob::UrlGetJob(String url) : ThreadPoolJob("UrlGetJob"), m_Url(url)
    {}

    UrlGetJob::~UrlGetJob()
    {}

    ThreadPoolJob::JobStatus UrlGetJob::runJob()
    {
        DBG("Getting URL: " + m_Url);

        URL url(m_Url);
        InputStream* inputStream = url.createInputStream(false, nullptr, nullptr, String::empty, cTimeout, nullptr);
        if (inputStream != nullptr)
        {
            String result = inputStream->readEntireStreamAsString();
            delete inputStream;
        }
        return jobHasFinished;
    }

Which works fine when there is an internet connection. However, when there isn’t a connection on OSX (still need to try in Windows), after a while this provokes an assertion failure in stopThread:

void Thread::stopThread (const int timeOutMilliseconds)
{
    // agh! You can't stop the thread that's calling this method! How on earth
    // would that work??
    jassert (getCurrentThreadId() != getThreadId()); // <----------- here!
...
}

More particular, the stopThread comes from juce_mac_Network.mm:

- (void) dealloc
{
    [self stop]; // <----------- here

    deleteAndZero (runLoopThread);
    [connection release];
    [data release];
    [dataLock release];
    [request release];
    [headers release];
    [super dealloc];
}

I don’t think that we’re doing something “special”, so I’m wondering if anyone has any ideas about this! I suppose the problem might arise from using the thread pool job? Is there a better way to do this?

greetings,

  • bram

#2

I just refactored that code this morning… Can you give it a shot with the new version and let me know if it helps! If not, I’ll take a deeper look at it.


#3

Thx for the quick reply… Build failed though:

juce_mac_NSViewComponentPeer.mm:109:38: error: cannot find protocol declaration for 'NSWindowDelegate'
            [window setDelegate: (id<NSWindowDelegate>) window];

juce_mac_MainMenu.mm:299:29: error: cannot find protocol declaration for 'NSMenuDelegate'
        [m setDelegate: (id<NSMenuDelegate>) callback];

#4

Gah… Are you using an old OSX SDK?


#5

Ok, try again now…


#6

Building… built.
(10.5 SDK)

Will report back if error still occurs.

  • bram

#7

Hey Jules,

Same error occurred… I have to add though that this is a very, very sporadic error, so it’s pretty hard to debug/find!
I’m going to make a tiny test app with only my URL getting job to see if I can reproduce it more faithfully.
Let me know if you find anything, I’ll report back when I have that app…

  • bram

#8

Jules,

below is a simple test app to reproduce… While doing this test I uncovered another error for you to have a look at… If you run this code multiple times (tested on windows for now), you sporadically get:

JUCE Assertion failure in d:\dev\juce\modules\juce_core\memory\juce_leakedobjectdetector.h, line JUCE Assertion failure in d:\dev\juce\modules\juce_core\memory\juce_leakedobjectdetector.h, line 71
JUCE Assertion failure in d:\dev\juce\modules\juce_core\memory\juce_leakedobjectdetector.h, line 71
JUCE Assertion failure in d:\dev\juce\modules\juce_core\memory\juce_leakedobjectdetector.h, line 71
71
<more of the same>
JUCE Assertion failure in d:\dev\juce\modules\juce_core\memory\juce_leakedobjectdetector.h, line 71
JUCE Assertion failure in d:\dev\juce\modules\juce_core\memory\juce_referencecountedobject.h, line 84

Here’s the simple app I’m using to uncover both the OSX bug and this leak bug (run with your network disconnected!):

#include "JuceHeader.h"

class UrlGetJob : public ThreadPoolJob
{
public:
    UrlGetJob(String url) : ThreadPoolJob("UrlGetJob"), m_Url(url)
    {

    };

    ~UrlGetJob()
    {

    };

    virtual ThreadPoolJob::JobStatus runJob()
    {
        URL url(m_Url);
        InputStream* inputStream = url.createInputStream(false, nullptr, nullptr, String::empty, cTimeout, nullptr);
        if (inputStream != nullptr)
        {
            String result = inputStream->readEntireStreamAsString();
            delete inputStream;
        }
        return jobHasFinished;
    };

private:
    String m_Url;
    static const int cTimeout = 1000;
};


//==============================================================================
int main (int argc, char* argv[])
{
    ThreadPool pool;

    for (int i=0; i<2000; i++)
    {
        pool.addJob(new UrlGetJob("http://www.somethingrandomhere123456.com"), true);
    }

    Thread::sleep(10000);

    return 0;
}

#9

Thanks for the test code!

It showed up some terrible old crap in that class… I’ve given it an overhaul now, so try it again!


#10

On OSX: yup, looks like its fixed!
On windows: still getting the mem leaks ( JUCE Assertion failure in d:\dev\juce\modules\juce_core\memory\juce_leakedobjectdetector.h, line 71 ) - but don’t really care about that right now :slight_smile:

Thanks jules,

  • bram

#11

The windows error is a good one!

I did manage to reproduce it, but I don’t think It’s a real bug. What I think is happening is this:

You simultaneously kick off a bunch of threads which all try to create a URL, which involves MemoryBlocks, HeapBlocks etc. Because none of those classes have been created before at this point in your program, your threads are all in a race to be the first to use them. That’s normally fine BUT each class’s leak detector has a function-local static counter that all these threads need to use. On OSX, the compiler automatically locks the creation of all static objects, but on windows, it doesn’t. So I think what you’re seeing is the leak-counters getting messed up when two threads overlap in initialising one of these static counters.

It’s easy enough to see if I’m right, by just doing a dummy URL open/close BEFORE you create your thread pool - that will make sure that all the statics are safely ready to use.


#12

If I run this enough times, I still get the warning… :slight_smile:

int main (int argc, char* argv[])
{
    URL url("http://www.google.com");
    InputStream* inputStream = url.createInputStream(false, nullptr, nullptr, String::empty, 200, nullptr);
    if (inputStream != nullptr)
    {
        String result = inputStream->readEntireStreamAsString();
        delete inputStream;
    }

    ThreadPool pool;

    for (int i=0; i<2000; i++)
    {
        pool.addJob(new UrlGetJob("http://www.somethingrandomhere123456.com"), true);
    }
    
    Thread::sleep(1000);

    return 0;
}

#13

(but as I said, for me this is… besides the matter now as it does all work…)

  • bram

#14

But your dummy code is opening a different URL to your threads, so the code-path could be different, and different classes may be used internally…


#15

same with using the same URL… I can reproduce this one as long as I start the program enough times:

although now it does look I’m having a hard time producing this error from before:

  • bram