Hi Matthias,

osg::notify() is written on the assumption that stream are thread
safe, avoiding adding extra mutexes is desirable as the performance
would really suffer if we had to add them to all osg::notify() calls.

I can see how that osg::initNotifyLevel() method might be problem if
multiple threads all call osg::notify() in parallel.  To avoid this
perhaps a local proxy object to force initialization would be
sufficient so that it initialized prior to any calls to osg::notify().
 To see if this works I've added the following to Notify.cpp:

struct InitNotifyProxy
{
    InitNotifyProxy()
    {
        osg::initNotifyLevel();
    }
};

static InitNotifyProxy s_initNotifyProxy;

The modified file is attached, could you try this out?

Robert.

On 7 June 2012 09:32, Matthias Schütze <matthi.schue...@googlemail.com> wrote:
> Hi,
>
> I am actually working on a complex project which uses OSG 3.0.1 on a
> Windows 7 Professional x64 SP1 platform with Visual Studio 2010
> Premium SP1. Because I encountered random heap corruption errors, I
> was testing and debugging with different configurations of my program
> and could state the following:
> - the invalid heap occurs mostly around one of the notification strings
> - the problem occurs with all threading models except SingleThreaded
> - the problem occurs still with an "empty" osg::NotifyHandler subclass
> (which originally implements some custom logging mechanism)
> - the problem occurs still without a custom osg::NotifyHandler
> subclass, i.e. when using osg::StandardNotifyHandler by default
>
> After all, I found the following forum thread which describes very
> similar experiences:
> http://forum.openscenegraph.org/viewtopic.php?t=9475 According to this
> forum thread, I designed my custom osg::NotifyHandler subclass
> thread-safe. Also, I checked out not to mix up incompatible binaries.
> My actual working project is not portable. But in order to track the
> problem on a non-Windows platform, I created another simple program
> which is inspired by the forum thread mentioned above. Here it is:
>
> #include <ctime>
> #include <cstdlib>
> #include <fstream>
> #include <OpenThreads/Thread>
> #include <osg/Notify>
>
> // empty logging handler
> class LogFileHandler : public osg::NotifyHandler
> {
> public:
>        LogFileHandler(const char *filename) {  }
>        void notify(osg::NotifySeverity severity, const char *message) {  }
>
> protected:
>        ~LogFileHandler() {  }
> };
>
> // simple worker thread
> class MyThread : public OpenThreads::Thread
> {
> public:
>        MyThread(int id)
>                : m_id(id)
>        {
>                OSG_INFO << "CREATE THREAD " << m_id << std::endl;
>        }
>
> protected:
>        virtual void run()
>        {
>                int lines = 100000; // number of notifications generated by 
> this thread
>                for (int i = 0; i < lines; ++i)
>                {
>                        // generate a random notification, e.g. "HELLO FROM 
> THREAD 2:
> aaabbbcccccdde[...]zz..."
>                        OSG_INFO << "HELLO FROM THREAD " << m_id << ": ";
>                        int letters = 26;
>                        for (int j = 0; j < letters; ++j)
>                        {
>                                char c = 'a' + j;
>                                switch (std::rand() % 5 + 1)
>                                {
>                                case 1: OSG_INFO << c; break;
>                                case 2: OSG_INFO << c << c; break;
>                                case 3: OSG_INFO << c << c << c; break;
>                                case 4: OSG_INFO << c << c << c << c; break;
>                                case 5: OSG_INFO << c << c << c << c << c; 
> break;
>                                }
>                        }
>                        OSG_INFO << "..." << std::endl;
>                }
>        }
>
> private:
>        int m_id;
> };
>
> // simple program
> int main(int argc, char **argv)
> {
>        srand(time(NULL));
>        osg::setNotifyLevel(osg::DEBUG_FP);
>        osg::setNotifyHandler(new LogFileHandler("log.txt")); // comment out
> this line: stdout will contain mixed up strings
>
>        const int count = 2; // number of threads accessing the notification
> API concurrently
>        MyThread *threads[count] = {  };
>        for (int i = 0; i < count; ++i)
>        {
>                threads[i] = new MyThread(i);
>                threads[i]->start();
>        }
>        for (int i = 0; i < count; ++i)
>        {
>                while (threads[i]->isRunning()) { /* active wait until the 
> worker
> thread ends */ }
>                delete threads[i];
>                threads[i] = NULL;
>        }
>
>        return EXIT_SUCCESS;
> }
>
> The program essentially imitates a multi-threaded environment for the
> notification API by creating some worker threads. Every thread
> generates a bulk of random notifications and pushes them per OSG_INFO.
> I think, one can compare this scenario with the case where a verbose
> notify level is used in combination with a non-SingleThreaded
> threading model, or am I wrong?
> In my Win 7 x64 VS 2010 environment the result is the following: I got
> frequent heap corruption errors. The problem can be reproduced more or
> less depending on the number of concurrently running threads and if it
> is a debug or release build. If I comment out the use of the custom
> osg::NotifyHandler subclass (which defaults to
> osg::StandardNotifyHandler), I recognized mixed up strings and missing
> endl's in stdout.
> Furthermore, I tested this program with Lubuntu 12.0.4 x64 and the
> QtCreator from the QtSDK 4.8.1. At least in debug mode, I got frequent
> heap corruption errors. If I comment out the use of the custom
> osg::NotifyHandler subclass, I recognized mixed up strings (debug) and
> missing endl's (debug and release) in stdout as well.
>
> Concluding, for my actual working project I will run SingleThreaded to
> avoid heap corruption as it fits my needs at the moment. But I cannot
> overcome some questions:
> - Do I miss some well-known restrictions, when using the notification
> API with multi-threading?
> - IMHO the heap corruption is caused by the non-thread-safe access to
> the global static instance of osg::NotifyStream defined in Notify.cpp.
> All over OSG, it is accessed by a reference to std::ostream which is
> returned by the osg::notify() function in order to call operator << on
> it. Can someone check these facts and explain to me, why there is no
> locking mechanism or the like to make the stream thread-safe?
>
> I appreciate every enlightenment or correction!
>
> Matthias Schütze, Germany
> _______________________________________________
> osg-users mailing list
> osg-users@lists.openscenegraph.org
> http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
/* -*-c++-*- OpenSceneGraph - Copyright (C) 1998-2006 Robert Osfield
 *
 * This library is open source and may be redistributed and/or modified under
 * the terms of the OpenSceneGraph Public License (OSGPL) version 0.0 or
 * (at your option) any later version.  The full license is in LICENSE file
 * included with this distribution, and on the openscenegraph.org website.
 *
 * This library is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * OpenSceneGraph Public License for more details.
*/
#include <osg/Notify>
#include <osg/ApplicationUsage>
#include <osg/ref_ptr>
#include <string>
#include <stdlib.h>
#include <stdio.h>
#include <sstream>
#include <iostream>

#include <ctype.h>

namespace osg
{

class NullStreamBuffer : public std::streambuf
{
private:
    std::streamsize xsputn(const std::streambuf::char_type *str, std::streamsize n)
    {
        return n;
    }
};

struct NullStream : public std::ostream
{
public:
    NullStream():
        std::ostream(new NullStreamBuffer)
    { _buffer = dynamic_cast<NullStreamBuffer *>(rdbuf()); }

    ~NullStream()
    {
        rdbuf(0);
        delete _buffer;
    }

protected:
    NullStreamBuffer* _buffer;
};

/** Stream buffer calling notify handler when buffer is synchronized (usually on std::endl).
 * Stream stores last notification severity to pass it to handler call.
 */
struct NotifyStreamBuffer : public std::stringbuf
{
    NotifyStreamBuffer() : _severity(osg::NOTICE)
    {
    }

    void setNotifyHandler(osg::NotifyHandler *handler) { _handler = handler; }
    osg::NotifyHandler *getNotifyHandler() const { return _handler.get(); }

    /** Sets severity for next call of notify handler */
    void setCurrentSeverity(osg::NotifySeverity severity) { _severity = severity; }
    osg::NotifySeverity getCurrentSeverity() const { return _severity; }

private:

    int sync()
    {
        sputc(0); // string termination
        if (_handler.valid())
            _handler->notify(_severity, pbase());
        pubseekpos(0, std::ios_base::out); // or str(std::string())
        return 0;
    }

    osg::ref_ptr<osg::NotifyHandler> _handler;
    osg::NotifySeverity _severity;
};

struct NotifyStream : public std::ostream
{
public:
    NotifyStream():
        std::ostream(new NotifyStreamBuffer)
    { _buffer = dynamic_cast<NotifyStreamBuffer *>(rdbuf()); }

    void setCurrentSeverity(osg::NotifySeverity severity)
    {
        _buffer->setCurrentSeverity(severity);
    }

    osg::NotifySeverity getCurrentSeverity() const
    {
        return _buffer->getCurrentSeverity();
    }

    ~NotifyStream()
    {
        rdbuf(0);
        delete _buffer;
    }

protected:
    NotifyStreamBuffer* _buffer;
};

}

using namespace osg;

static osg::ApplicationUsageProxy Notify_e0(osg::ApplicationUsage::ENVIRONMENTAL_VARIABLE, "OSG_NOTIFY_LEVEL <mode>", "FATAL | WARN | NOTICE | DEBUG_INFO | DEBUG_FP | DEBUG | INFO | ALWAYS");

static bool s_NeedNotifyInit = true;
static osg::NotifySeverity g_NotifyLevel = osg::NOTICE;
static osg::NullStream *g_NullStream;
static osg::NotifyStream *g_NotifyStream;

struct InitNotifyProxy
{
    InitNotifyProxy()
    {
        osg::initNotifyLevel();
    }
};

static InitNotifyProxy s_initNotifyProxy;

void osg::setNotifyLevel(osg::NotifySeverity severity)
{
    if (s_NeedNotifyInit) osg::initNotifyLevel();
    g_NotifyLevel = severity;
}


osg::NotifySeverity osg::getNotifyLevel()
{
    if (s_NeedNotifyInit) osg::initNotifyLevel();
    return g_NotifyLevel;
}

void osg::setNotifyHandler(osg::NotifyHandler *handler)
{
    osg::NotifyStreamBuffer *buffer = static_cast<osg::NotifyStreamBuffer *>(g_NotifyStream->rdbuf());
    if (buffer)
        buffer->setNotifyHandler(handler);
}

osg::NotifyHandler* osg::getNotifyHandler()
{
    if (s_NeedNotifyInit) osg::initNotifyLevel();
    osg::NotifyStreamBuffer *buffer = static_cast<osg::NotifyStreamBuffer *>(g_NotifyStream->rdbuf());
    return buffer ? buffer->getNotifyHandler() : 0;
}

bool osg::initNotifyLevel()
{
    static osg::NullStream s_NullStream;
    static osg::NotifyStream s_NotifyStream;

    g_NullStream = &s_NullStream;
    g_NotifyStream = &s_NotifyStream;

    // g_NotifyLevel
    // =============

    g_NotifyLevel = osg::NOTICE; // Default value

    char* OSGNOTIFYLEVEL=getenv("OSG_NOTIFY_LEVEL");
    if (!OSGNOTIFYLEVEL) OSGNOTIFYLEVEL=getenv("OSGNOTIFYLEVEL");
    if(OSGNOTIFYLEVEL)
    {

        std::string stringOSGNOTIFYLEVEL(OSGNOTIFYLEVEL);

        // Convert to upper case
        for(std::string::iterator i=stringOSGNOTIFYLEVEL.begin();
            i!=stringOSGNOTIFYLEVEL.end();
            ++i)
        {
            *i=toupper(*i);
        }

        if(stringOSGNOTIFYLEVEL.find("ALWAYS")!=std::string::npos)          g_NotifyLevel=osg::ALWAYS;
        else if(stringOSGNOTIFYLEVEL.find("FATAL")!=std::string::npos)      g_NotifyLevel=osg::FATAL;
        else if(stringOSGNOTIFYLEVEL.find("WARN")!=std::string::npos)       g_NotifyLevel=osg::WARN;
        else if(stringOSGNOTIFYLEVEL.find("NOTICE")!=std::string::npos)     g_NotifyLevel=osg::NOTICE;
        else if(stringOSGNOTIFYLEVEL.find("DEBUG_INFO")!=std::string::npos) g_NotifyLevel=osg::DEBUG_INFO;
        else if(stringOSGNOTIFYLEVEL.find("DEBUG_FP")!=std::string::npos)   g_NotifyLevel=osg::DEBUG_FP;
        else if(stringOSGNOTIFYLEVEL.find("DEBUG")!=std::string::npos)      g_NotifyLevel=osg::DEBUG_INFO;
        else if(stringOSGNOTIFYLEVEL.find("INFO")!=std::string::npos)       g_NotifyLevel=osg::INFO;
        else std::cout << "Warning: invalid OSG_NOTIFY_LEVEL set ("<<stringOSGNOTIFYLEVEL<<")"<<std::endl;

    }

    // Setup standard notify handler
    osg::NotifyStreamBuffer *buffer = dynamic_cast<osg::NotifyStreamBuffer *>(g_NotifyStream->rdbuf());
    if (buffer && !buffer->getNotifyHandler())
        buffer->setNotifyHandler(new StandardNotifyHandler);

    s_NeedNotifyInit = false;

    return true;

}

#ifndef OSG_NOTIFY_DISABLED
bool osg::isNotifyEnabled( osg::NotifySeverity severity )
{
    if (s_NeedNotifyInit) osg::initNotifyLevel();
    return severity<=g_NotifyLevel;
}
#endif

std::ostream& osg::notify(const osg::NotifySeverity severity)
{
    if (s_NeedNotifyInit) osg::initNotifyLevel();

    if (osg::isNotifyEnabled(severity))
    {
        g_NotifyStream->setCurrentSeverity(severity);
        return *g_NotifyStream;
    }
    return *g_NullStream;
}

void osg::StandardNotifyHandler::notify(osg::NotifySeverity severity, const char *message)
{
#if 1
    if (severity <= osg::WARN)
        fputs(message, stderr);
    else
        fputs(message, stdout);
#else
   fputs(message, stdout);
#endif
}

#if defined(WIN32) && !defined(__CYGWIN__)

#ifndef WIN32_LEAN_AND_MEAN
    #define WIN32_LEAN_AND_MEAN
#endif
#include <windows.h>

void osg::WinDebugNotifyHandler::notify(osg::NotifySeverity severity, const char *message)
{
    OutputDebugStringA(message);
}

#endif
_______________________________________________
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org

Reply via email to