Multithreaded static initialization bugs in Inventor plugin

Bug #1211993 reported by Christopher R. Baker on 2013-08-13
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
openscenegraph (Ubuntu)
Undecided
Unassigned

Bug Description

There are three instances of a classical method-local-static multithreaded initialization bug in the Inventor plugin for OSG that trigger various memory faults when reading multiple VRML files in parallel via osgDB::readNodeFile. These bugs are of the form:

static std::map<Stuff,OtherStuff> myHandyMap;
static bool once = true;
if(once) { ...fill myHandyMap; once = false }
... use myHandyMap;

To repeat: try loading multiple VRML files from multiple threads. The liklihood of the bug depends on many factors, but my applicaiton, which parallel-loads some dozens of small (<100K) VRML files on startup, triggers this problem 25% of the time or more.

The attached patch (inventor-plugin-multithread.patch) rectifies this problem by:
1 - Inheriting MyHandyMap from std::map, then
2 - Moving the map initialization into the derived constructor, which
3 - Is intrinsically protected from multithread issues by g++ (and is part of the C++ standard), unless you pass -fno-threadsafe-statics, which is strongly discouraged by the man page.

This patch was made via and may be appended to the quilt series for this package.

A peek at the OSG trunk indicates that this problem persists upstream as well:
http://trac.openscenegraph.org/projects/osg/browser/OpenSceneGraph/trunk/src/osgPlugins/Inventor/ConvertFromInventor.cpp

Should I also submit this upstream, or will that happen naturally as part of the ubuntu package ecosystem?

Of note, a similar, but more sinister set of multithreading bugs exist in the actual VRML plugin, but this plugin is not built as part of the ubuntu package, as it seems to depend on the no-longer-supported OpenVRML packages. I also have an untested patch that stabs at these problems if it is worth providing.

Alberto Luaces (azdo-b) wrote :

Hi Christopher, I am one of the guys packaging OSG in Debian. Thank you for your patch.

Yes, it would be preferable to send the patch upstream; in fact I was about to submit it there on your behalf. However, I tried to apply your patch to the current upstreams SVN, and it does not apply cleanly. From version 3.2.0, some osg::Geometry methods were deprecated, and thus the VRML plugin has changed as well. Could you refresh your patch against the latest version, please?

I have attached a refreshed patch against OSG trunk (rev 13788). It compiles and is visually accurate, but is otherwise untested.

Even if this patch is incorporated upstream, I would prefer if the original patch were applied to the version of OSG that is discributed with 12.04 LTS, as my project is tracking the LTS releases and I would rather not keep around my marginally-modified version of OSG.

As a new concern, however, fixing this multithreaded initialization bug has unmasked another: that the library that the OSG Inventor plugin adapts (libcoin60/source package coin3) is not built (at least on 12.04) with the configuration option "--enable-threadsafe". Without that option, the OSG usage pattern in the Inventor plugin triggers another static-global-initialization problem inside libcoin, so I still occasionally race and segfault on the first batch load anyway... yeesh...

Rebuilding libcoin60 with that extra configuration flag enabled seems to relieve the problem without appreciable performance penalty, but I don't know if it is appropriate to ask the LTS maintainers to just go changing config options in a package like that. I know that there have previously been "-mt" variants of such packages, but no such beast seems to exist for libcoin.

The attachment "Patch against svn trunk" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

tags: added: patch
Alberto Luaces (azdo-b) wrote :

Sorry for the late answer, I thought that the tracking system would
email me any response.

I have submitted your patch to upstream[1], and, in the meantime, I
will also include it in the next release for Debian, so it should be
ported to Ubuntu as well. I have also filled a bug for libcoin in
order for them to enable the multithreading support[2], that I also
suppose will eventually land into Ubuntu version.

Nevertheless, I don't know if this will finally reach 12.04 or if it
is only to be applied to future versions.

Thanks!

Footnotes:
[1] http://lists.openscenegraph.org/pipermail/osg-submissions-openscenegraph.org/2013-September/010635.html

[2] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=723567

Excellent! Thanks for pushing this through!

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package openscenegraph - 3.2.0~rc1-4

---------------
openscenegraph (3.2.0~rc1-4) unstable; urgency=low

  * Added patch for fixing a multithreading initialization bug in the
    inventor plugin (LP: #1211993).
  * Closing bug about conflicting files in 3.2.0-rc1 and 3.2.0 since next
    version to be packaged is 3.2.1 (Closes: #722674).

 -- Alberto Luaces Fernández <email address hidden> Mon, 17 Feb 2014 23:36:07 +0100

Changed in openscenegraph (Ubuntu):
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.