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 <[email protected]> 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
> [email protected]
> 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
[email protected]
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org