Re: [osg-users] Text Crash On Main Thread While DB Pager Unrefs Font From Temp Object Cache
Hi Ryan, On Wed, Dec 16, 2009 at 9:43 AM, Ryan H. Kawicki wrote: > It might not be convincing to have to go back to the code base and remove > these unsafe sections of code, but by having a set of documentation is really > not going to help eighty or ninety percent of the OSG community. I rarely > look at the documentation. My documentation is really just the source code > that is provided. With multiple cores available on the cheap and new and old > applications requiring database support, I feel that it is in the best > interest of OSG to have these functions removed. It will save for headaches > in the long run for you, as you will not have to explain each and every time > to use the ref version instead of the other. I understand the principle of short term pain for long term gain... but... In the current case I'm looking towards getting the OSG users moving on to the upcoming 3.0, during this transition their will inevitably be some refactor work required by users due to the wider changes such as GLES/GL3 support, but the more pain there is the slower the uptake will be and the slower we'll sort out all the bugs/issues. In this context I want to minimize how much more pain we might introduce. The less pain the quicker people will try out and move to OSG 3.x and the quicker it's quality will improve. So right now I don't want to add any further backwards compatibility breakages if I can avoid it. >> The cache flipping trick used is not good needs to be removed. I'd be >> inclined towards use osgDB::Options to store the cache, and have an in >> memory ObjectCache to mirror the current FileCache. Such an approach >> would allow you to override the default ObjectCache to a single read >> call, such as ones done from the DatabasePager and avoid the need to >> cache flipping completely. >> > > Are there plans for this to be addressed for a future release? If so, can > you detail which release you are trying to target? I don't have any specific version for this. It's an issue right now and should be fixed a soon as I or others have time to tackle it. I would guess that it's a day or two's work. Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Text Crash On Main Thread While DB Pager Unrefs Font From Temp Object Cache
> > More generally I'm not convinced that we need to deprecate the none > ref versions. I'd be inclined towards just documenting that > multi-threaded reads should use the Ref versions. > It might not be convincing to have to go back to the code base and remove these unsafe sections of code, but by having a set of documentation is really not going to help eighty or ninety percent of the OSG community. I rarely look at the documentation. My documentation is really just the source code that is provided. With multiple cores available on the cheap and new and old applications requiring database support, I feel that it is in the best interest of OSG to have these functions removed. It will save for headaches in the long run for you, as you will not have to explain each and every time to use the ref version instead of the other. > > The cache flipping trick used is not good needs to be removed. I'd be > inclined towards use osgDB::Options to store the cache, and have an in > memory ObjectCache to mirror the current FileCache. Such an approach > would allow you to override the default ObjectCache to a single read > call, such as ones done from the DatabasePager and avoid the need to > cache flipping completely. > Are there plans for this to be addressed for a future release? If so, can you detail which release you are trying to target? Thanks, Ryan -- Read this topic online here: http://forum.openscenegraph.org/viewtopic.php?p=21553#21553 ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Text Crash On Main Thread While DB Pager Unrefs Font From Temp Object Cache
Hi Ryan, On Mon, Dec 14, 2009 at 10:38 AM, Ryan H. Kawicki wrote: > Observation 1: > Using the unsafe versions is bad. Now we know and knowing is half the > battler, but should this be take further. It might be a good idea to tag the > unsafe versions as deprecated, this way a compiler warning can be generated. > This can be rolled into the 2.10 release of OSG. I would then say that a > version of two later, they can be removed all together. Within the OSG we should certainly move across to use the ref versions internally in NodeKits and plugins. More generally I'm not convinced that we need to deprecate the none ref versions. I'd be inclined towards just documenting that multi-threaded reads should use the Ref versions. > Observation 2: > It looks like if you mix cached and uncached operations on different threads, > this is the chance that cached objects will be lost by the registry. I'm not > sure how to overcome this issue, and this issue might need a more trained eye > and someone with greater knowledge of OSGs internal workings. I'll leave > that up to you and the community to decide this one. The cache flipping trick used is not good needs to be removed. I'd be inclined towards use osgDB::Options to store the cache, and have an in memory ObjectCache to mirror the current FileCache. Such an approach would allow you to override the default ObjectCache to a single read call, such as ones done from the DatabasePager and avoid the need to cache flipping completely. > Observation 3: > I did try to read osgText::Font objects using a noncached call, but this was > a very bad thing to do. It basically made our entire terrain page in bad > texture data. I'm not sure what the problem was there, but our terrain was > paging in with some pink textures. In any case, I did notice something quite > strange or even bad. When caching of the font objects was off, calling to > read the font object internally was caching this data in > FreeTypeLibrary::getFont. The _fontImplementationSet continually was growing > and growing. With caching turned on, the _fontImplementationSet averaged > around nine objects, with some of them being the same. I can only surmise > that this was due to the object cache problem I described above and would > fall to the same fate as using the noncached version. In any case, I wanted > to point this out, as there might be some optimizations that can be placed > here. Font's really do need to be cached and shared between instanced, perhaps it even warrant's it's own separate ObjectCache? Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Text Crash On Main Thread While DB Pager Unrefs Font From Temp Object Cache
Robert,
I just wanted to respond with my investigations and observations.
>
> I sounds like the lack of a readRefFontFile() is a potential problem.
> In the case of include/osgDB/ReadFile there are readRefNodeFile() etc.
> functions that pass back a ref_ptr<> rather than a C pointer, and are
> all safer in multi-thread situations where object cache is in play.
> Could you try implementing a readRefFontFile() and convert osgText
> across to use this to see if it helps.
>
So, I am mistaken and there is a readRefFontFile already implemented. All I
needed to do was look a few lines down. I've changed my code to use this
function instead and it seems to have solved the issue at hand. I will go into
more detail about this, as I've done some really deep digging into this
There are a few places that should more than likely be changed within OSG.
TXPArchive.cpp and ScalarBar.cpp both use the unsafe version. It will more
than like be quicker for you to fix this issue than for me to submit source to
the submission. I'll just leave that one up to you.
Investigation:
So, I dug into this problem almost all day on Saturday. I think I have a good
understanding of what this problem is. I also believe that there is another
problem that needs to be addressed. More to come on that.
In our application, we have a database pager thread that is requesting node
files in a non-cached manner. On our main thread, when requesting a
osgText::Font object, the underlying function is requesting these objects to be
cached. I'm posting the Registry::readImplementation below for quick reference.
So, lets say that our database pager has some requests. No remember that these
requests are not going to be cached. Once the Registry::readImplementation
function is reached, the else portion of the if/else statement is executed.
This portion of the code takes all objects in the _objectCache and swaps it
with a temporary local object cache.
Now, lets assume that the database pager is busy at work bringing in a new node
of data. The temporary data is all setup and the database pager is happily in
the readFunctor. Now, the main thread decides to request a font object. With
the database pager still in the readFunctor, the font object is able to get
inside of the if portion of the if/else statement. The object is inserted into
the _objectCache and happily moves on. As the main thread continues back down
the stack, one of the function reached first before getting to
osgText::readFontFile is osgDB::readObjectFile. This function basically calls
takeObject on the read results object, which basically adds then subtracts a
reference from the returning object. The point I want to make is that the
reference is 1. At this point, the stack is unwound a bit further to the
original calling function. It has a pure pointer and passes that along to the
osgText::setFont function.
Lets say that the database pager was finally able to return from doing its read
somewhere between the main threads execution of Registry::readImplementation
and osgText::setFont. Now, the database pager had done a swap of data on the
_objectCache and the temporary object cache. Now this means that the data in
_objectCache belongs to whatever was swapped in from the temporary object cache
of the database pager thread. The main thread decided to load some data and
cache the results. This data is now in the _objectCache, which belongs to the
temporary cache of the database pager. When the database pager returns, it
swaps again the two pointers. Now, the temporary cache contains the newly
cached object and the _objectCache contains nothing. We can see where this is
going to lead. As the temporary cache is destructed, the stored objects, our
font object, gets unreferenced. This makes for a sad story when trying to
access this data later.
Code:
ReaderWriter::ReadResult Registry::readImplementation(const ReadFunctor&
readFunctor, bool useObjectCache)
{
std::string file(readFunctor._filename);
if (useObjectCache)
{
// search for entry in the object cache.
{
OpenThreads::ScopedLock lock(_objectCacheMutex);
ObjectCache::iterator oitr=_objectCache.find(file);
if (oitr!=_objectCache.end())
{
notify(INFO)<<"returning cached instanced of "second.first.get(),
ReaderWriter::ReadResult::FILE_LOADED_FROM_CACHE);
else return ReaderWriter::ReadResult("Error file does not
contain an osg::Object");
}
}
ReaderWriter::ReadResult rr = read(readFunctor);
if (rr.validObject())
{
// update cache with new entry.
notify(INFO)<<"Adding to object cache "< lock(_objectCacheMutex);
tmpObjectCache.swap(_objectCache);
}
ReaderWriter::ReadResult rr = read(readFun
Re: [osg-users] Text Crash On Main Thread While DB Pager Unrefs Font From Temp Object Cache
Is it possible for the database pager thread to switch the cache to a temp
cache, and then another thread comes in and thinks that temp cache is the
actual cache? So when the database pager swaps the cache again, then that
ref_ptr in the tmp cache is destroyed? the other thread continues on
thinking the cache wasn't a temporary one.
Could get into a situation where multiple threads are switching the caches,
then the main cache may not be restored?
On Fri, Dec 11, 2009 at 7:53 AM, Robert Osfield wrote:
> Hi Ryan,
>
> I sounds like the lack of a readRefFontFile() is a potential problem.
> In the case of include/osgDB/ReadFile there are readRefNodeFile() etc.
> functions that pass back a ref_ptr<> rather than a C pointer, and are
> all safer in multi-thread situations where object cache is in play.
> Could you try implementing a readRefFontFile() and convert osgText
> across to use this to see if it helps.
>
> Cheers,
> Robert.
>
> On Fri, Dec 11, 2009 at 1:04 PM, Ryan H. Kawicki
> wrote:
> > I've been investigating a crash that has plagued us for quite some time.
> I've finally gotten some time to sit down and take a look at this.
> >
> > Details:
> > OSG: 2.8.1
> > OS: WinXP SP3 32Bit
> > CPU: 8 Core 2.5GHz Xenon
> > Graphics: Quadro FX 3700 512 MB
> > Memory: Plenty
> >
> > Description:
> > Our application embeds OSG into two views. There are multiple threads at
> work here, but the two of importance are the Map thread and the
> DatabasePager thread.
> >
> > At certain points during the execution of our application, the Map thread
> will be given a request to create an osgText object. One of the operations
> is to request a font for the osgText object. At this point we call down
> into osgText::readFontFile( ... ).
> >
> > At this same time, the database pager is hard at work reading paged files
> from disk on its own thread. At some point, the database pager gets into
> Registry::readImplementation( ... ) and begins to copy the object cache to a
> temp object cache. There is only one object in the cache and it is an
> osgText::Font object. At some point during this stage, an unref of the
> object happens. Here is where the oddity comes in. When the object is
> getting unreferenced, the code checks to see if the object needs to be
> deleted. For some odd reason this value is 0 on the DB thread but 2 on the
> Map thread. When the Map thread tries to use the font object, it dies, as
> the DB thread has already released it. I would give more details on this,
> but the computer I was working on did not come back from standby and
> everything I am stating is from memory. Sorry.
> >
> > Question:
> > Now that we have gotten through that, the question I have is related to
> the implementation of osgText::readFontFile. Would it not be safer to have
> this function use ref_ptr objects internally rather than standard pointers?
> I'm quite confused as to the design decisions behind this and hope someone
> can help me here.
> >
> >
> > Code:
> >
> > osgText::Font* osgText::readFontFile(const std::string& filename, const
> osgDB::ReaderWriter::Options* userOptions)
> > {
> >if (filename=="") return 0;
> >
> >std::string foundFile = findFontFile(filename);
> >if (foundFile.empty()) return 0;
> >
> >OpenThreads::ScopedLock lock(s_FontFileMutex);
> >
> >osg::ref_ptr localOptions;
> >if (!userOptions)
> >{
> >localOptions = new osgDB::ReaderWriter::Options;
> >
>
> localOptions->setObjectCacheHint(osgDB::ReaderWriter::Options::CACHE_OBJECTS);
> >}
> >
> >osg::Object* object = osgDB::readObjectFile(foundFile, userOptions ?
> userOptions : localOptions.get());
> >
> >// if the object is a font then return it.
> >osgText::Font* font = dynamic_cast(object);
> >if (font) return font;
> >
> >// otherwise if the object has zero references then delete it by doing
> another unref().
> >if (object && object->referenceCount()==0) object->unref();
> >return 0;
> > }
> >
> >
> >
> >
> > Thanks,
> > Ryan
> >
> > --
> > Read this topic online here:
> > http://forum.openscenegraph.org/viewtopic.php?p=21314#21314
> >
> >
> >
> >
> >
> > ___
> > osg-users mailing list
> > osg-users@lists.openscenegraph.org
> >
> http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
> >
> ___
> osg-users mailing list
> osg-users@lists.openscenegraph.org
> http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
>
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Text Crash On Main Thread While DB Pager Unrefs Font From Temp Object Cache
Hi Ryan,
I sounds like the lack of a readRefFontFile() is a potential problem.
In the case of include/osgDB/ReadFile there are readRefNodeFile() etc.
functions that pass back a ref_ptr<> rather than a C pointer, and are
all safer in multi-thread situations where object cache is in play.
Could you try implementing a readRefFontFile() and convert osgText
across to use this to see if it helps.
Cheers,
Robert.
On Fri, Dec 11, 2009 at 1:04 PM, Ryan H. Kawicki
wrote:
> I've been investigating a crash that has plagued us for quite some time.
> I've finally gotten some time to sit down and take a look at this.
>
> Details:
> OSG: 2.8.1
> OS: WinXP SP3 32Bit
> CPU: 8 Core 2.5GHz Xenon
> Graphics: Quadro FX 3700 512 MB
> Memory: Plenty
>
> Description:
> Our application embeds OSG into two views. There are multiple threads at
> work here, but the two of importance are the Map thread and the DatabasePager
> thread.
>
> At certain points during the execution of our application, the Map thread
> will be given a request to create an osgText object. One of the operations
> is to request a font for the osgText object. At this point we call down into
> osgText::readFontFile( ... ).
>
> At this same time, the database pager is hard at work reading paged files
> from disk on its own thread. At some point, the database pager gets into
> Registry::readImplementation( ... ) and begins to copy the object cache to a
> temp object cache. There is only one object in the cache and it is an
> osgText::Font object. At some point during this stage, an unref of the
> object happens. Here is where the oddity comes in. When the object is
> getting unreferenced, the code checks to see if the object needs to be
> deleted. For some odd reason this value is 0 on the DB thread but 2 on the
> Map thread. When the Map thread tries to use the font object, it dies, as
> the DB thread has already released it. I would give more details on this,
> but the computer I was working on did not come back from standby and
> everything I am stating is from memory. Sorry.
>
> Question:
> Now that we have gotten through that, the question I have is related to the
> implementation of osgText::readFontFile. Would it not be safer to have this
> function use ref_ptr objects internally rather than standard pointers? I'm
> quite confused as to the design decisions behind this and hope someone can
> help me here.
>
>
> Code:
>
> osgText::Font* osgText::readFontFile(const std::string& filename, const
> osgDB::ReaderWriter::Options* userOptions)
> {
> if (filename=="") return 0;
>
> std::string foundFile = findFontFile(filename);
> if (foundFile.empty()) return 0;
>
> OpenThreads::ScopedLock lock(s_FontFileMutex);
>
> osg::ref_ptr localOptions;
> if (!userOptions)
> {
> localOptions = new osgDB::ReaderWriter::Options;
>
> localOptions->setObjectCacheHint(osgDB::ReaderWriter::Options::CACHE_OBJECTS);
> }
>
> osg::Object* object = osgDB::readObjectFile(foundFile, userOptions ?
> userOptions : localOptions.get());
>
> // if the object is a font then return it.
> osgText::Font* font = dynamic_cast(object);
> if (font) return font;
>
> // otherwise if the object has zero references then delete it by doing
> another unref().
> if (object && object->referenceCount()==0) object->unref();
> return 0;
> }
>
>
>
>
> Thanks,
> Ryan
>
> --
> Read this topic online here:
> http://forum.openscenegraph.org/viewtopic.php?p=21314#21314
>
>
>
>
>
> ___
> osg-users mailing list
> osg-users@lists.openscenegraph.org
> http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
>
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
[osg-users] Text Crash On Main Thread While DB Pager Unrefs Font From Temp Object Cache
I've been investigating a crash that has plagued us for quite some time. I've
finally gotten some time to sit down and take a look at this.
Details:
OSG: 2.8.1
OS: WinXP SP3 32Bit
CPU: 8 Core 2.5GHz Xenon
Graphics: Quadro FX 3700 512 MB
Memory: Plenty
Description:
Our application embeds OSG into two views. There are multiple threads at work
here, but the two of importance are the Map thread and the DatabasePager thread.
At certain points during the execution of our application, the Map thread will
be given a request to create an osgText object. One of the operations is to
request a font for the osgText object. At this point we call down into
osgText::readFontFile( ... ).
At this same time, the database pager is hard at work reading paged files from
disk on its own thread. At some point, the database pager gets into
Registry::readImplementation( ... ) and begins to copy the object cache to a
temp object cache. There is only one object in the cache and it is an
osgText::Font object. At some point during this stage, an unref of the object
happens. Here is where the oddity comes in. When the object is getting
unreferenced, the code checks to see if the object needs to be deleted. For
some odd reason this value is 0 on the DB thread but 2 on the Map thread. When
the Map thread tries to use the font object, it dies, as the DB thread has
already released it. I would give more details on this, but the computer I was
working on did not come back from standby and everything I am stating is from
memory. Sorry.
Question:
Now that we have gotten through that, the question I have is related to the
implementation of osgText::readFontFile. Would it not be safer to have this
function use ref_ptr objects internally rather than standard pointers? I'm
quite confused as to the design decisions behind this and hope someone can help
me here.
Code:
osgText::Font* osgText::readFontFile(const std::string& filename, const
osgDB::ReaderWriter::Options* userOptions)
{
if (filename=="") return 0;
std::string foundFile = findFontFile(filename);
if (foundFile.empty()) return 0;
OpenThreads::ScopedLock lock(s_FontFileMutex);
osg::ref_ptr localOptions;
if (!userOptions)
{
localOptions = new osgDB::ReaderWriter::Options;
localOptions->setObjectCacheHint(osgDB::ReaderWriter::Options::CACHE_OBJECTS);
}
osg::Object* object = osgDB::readObjectFile(foundFile, userOptions ?
userOptions : localOptions.get());
// if the object is a font then return it.
osgText::Font* font = dynamic_cast(object);
if (font) return font;
// otherwise if the object has zero references then delete it by doing
another unref().
if (object && object->referenceCount()==0) object->unref();
return 0;
}
Thanks,
Ryan
--
Read this topic online here:
http://forum.openscenegraph.org/viewtopic.php?p=21314#21314
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
