Hi Pjotr, On 27 October 2014 10:58, Pjotr Svetachov <[email protected]> wrote:
> As said we have multiple files that reference the same image for a > texture, I have found two spots that can cause problems: > - In InputStream::readImage(bool readFromExternal) at line 777 the program > could get an image from the cache. After this it continues to read the > image fields and set parameters. The most problematic is at like 784 where > it will set the image filename, which is a string. This is not thread safe. > Looking further it will call readObjectFields which will read the name and > userdata and crash on that. > I've just read through the code, it certainly assumes that it's the only thread writing to the image data, and doesn't account for the possibility that when the osgDB::Registry object cache is active it is possible to start modifying an image that is already been actively used by another thread. I expect this is something that Rui Wang didn't pick up as a possibility when he originally wrote the serializers. In the case when the image is shared it looks to me like the best approach would be to not allow any additional modifications to the image by the InputStream::readImage() method. Even if you added mutex locks around the write operations to the mutex you'd ended with an image that flips state during it's lifetime so that the original owners of the image would then have an Image in a new state that it doesn't have baring on what it was originally. This might be OK in some instances, but could cause real problems for application/code that assumes the Image is invariant once it has been set up. Changing the behaviour to ignore the local modifications when an Image is shared is something that might break applications too, but my instinct is to treat Image as invariant once setup is the more common case so will affect users left often than the current state. Perhaps one possibility would be to have a flag in InputStream to give a hint to what to do. If we do ignore the extra write for Image's pulled from the cache we'd need to still read then discard the options to avoid breaking file compatibility. Another, more drastic workaround would be to duplicate the osg::Image from the cache and let the new Image be modified. This rather defaults the purpose of using a cache though. It's worth adding that this cached object issue will be a wider one that just Image handling, so whatever solution we plump for we'll need to tackle these other cases as well. > - When a Texture2D is read it will call Texture2D::setImage and this > function will call _image->addClient which can mess up the _numClients > counter. > > The second problem can also appear while reading ive files. I think this problem will be easier to resolve, changing the _numClients to an osg::Atomic should probably be sufficient. I will test this change right now to see if it has any effect on the crashes I'm seeing. Robert.
_______________________________________________ osg-users mailing list [email protected] http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
