Merge pull request #1127 from johnhaddon/dlopenFlagsPR

andrewkaufman · web-flow · commit 18e60affe067 · 2021-03-02T08:49:42.000-08:00
Add IECORE_RTLD_GLOBAL environment variable
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -44,7 +44,7 @@ jobs:
             buildType: RELEASE
             containerImage: gafferhq/build:1.0.0
             options: .github/workflows/main/options.posix
-            dependenciesURL: https://github.com/GafferHQ/dependencies/releases/download/2.1.1/gafferDependencies-2.1.1-Python2-linux.tar.gz
+            dependenciesURL: https://github.com/GafferHQ/dependencies/releases/download/3.0.0a2/gafferDependencies-3.0.0-Python2-linux.tar.gz
             tests: testCorePython testScene testImage testAlembic testUSD testVDB testArnold testAppleseed
             publish: true
 
@@ -53,7 +53,7 @@ jobs:
             buildType: DEBUG
             containerImage: gafferhq/build:1.0.0
             options: .github/workflows/main/options.posix
-            dependenciesURL: https://github.com/GafferHQ/dependencies/releases/download/2.1.1/gafferDependencies-2.1.1-Python2-linux.tar.gz
+            dependenciesURL: https://github.com/GafferHQ/dependencies/releases/download/3.0.0a2/gafferDependencies-3.0.0-Python2-linux.tar.gz
             tests: testCorePython testScene testImage testAlembic testUSD testVDB testArnold testAppleseed
             publish: false
 
@@ -62,7 +62,7 @@ jobs:
             buildType: RELEASE
             containerImage: gafferhq/build:1.0.0
             options: .github/workflows/main/options.posix
-            dependenciesURL: https://github.com/GafferHQ/dependencies/releases/download/2.1.1/gafferDependencies-2.1.1-Python3-linux.tar.gz
+            dependenciesURL: https://github.com/GafferHQ/dependencies/releases/download/3.0.0a2/gafferDependencies-3.0.0-Python3-linux.tar.gz
             tests: testCorePython testScene testImage testAlembic testUSD testVDB testArnold testAppleseed
             publish: true
 
@@ -71,7 +71,7 @@ jobs:
             buildType: RELEASE
             containerImage:
             options: .github/workflows/main/options.posix
-            dependenciesURL: https://github.com/GafferHQ/dependencies/releases/download/2.1.1/gafferDependencies-2.1.1-Python2-osx.tar.gz
+            dependenciesURL: https://github.com/GafferHQ/dependencies/releases/download/3.0.0a2/gafferDependencies-3.0.0-Python2-osx.tar.gz
             # `testArnold` currently omitted due to problems loading `libai.dylib` on MacOS workers (possible SIP issue),
             # and `testAppleseed` due to clashes with system image IO frameworks.
             tests: testCorePython testScene testImage testAlembic testUSD testVDB
@@ -164,6 +164,8 @@ jobs:
     - name: Test
       run: |
         scons ${{ matrix.tests }} BUILD_TYPE=${{ matrix.buildType }} OPTIONS=${{ matrix.options }} CXXSTD=c++14 BUILD_CACHEDIR=sconsCache
+      env:
+        IECORE_RTLD_GLOBAL: 0
       # We have some work to do before we can run the tests
       # successfully on Windows. Until then, we satisfy ourselves
       # with checking that it at least _builds_.
diff --git a/.github/workflows/main/options.posix b/.github/workflows/main/options.posix
@@ -66,7 +66,7 @@ APPLESEED_LIB_PATH = deps + "/appleseed/lib"
 # Environment
 # ===========
 
-ENV_VARS_TO_IMPORT = "PATH"
+ENV_VARS_TO_IMPORT = "PATH IECORE_RTLD_GLOBAL"
 
 if platform.system() == "Darwin" :
 	os.environ["DYLD_FRAMEWORK_PATH"] = libs
diff --git a/SConstruct b/SConstruct
@@ -2995,6 +2995,10 @@ arnoldPythonModuleEnv.Append(
 arnoldDriverEnv = arnoldEnv.Clone( IECORE_NAME = "ieOutputDriver" )
 arnoldDriverEnv["SHLIBPREFIX"] = ""
 arnoldDriverEnv["SHLIBSUFFIX"] = ".so" if env["PLATFORM"] != "win32" else ".dll"
+if env["PLATFORM"]=="darwin" :
+	# Symbols we need from `libai.dylib` will be resolved at runtime when Arnold
+	# loads the driver.
+	arnoldDriverEnv.Append( LINKFLAGS = "-Wl,-undefined,dynamic_lookup" )
 
 haveArnold = False
 
@@ -3041,7 +3045,6 @@ if doConfigure :
 
 		arnoldDriverEnv.Append(
 			LIBS = [
-				"ai",
 				os.path.basename( coreEnv.subst( "$INSTALL_LIB_NAME" ) ),
 				os.path.basename( imageEnv.subst( "$INSTALL_LIB_NAME" ) ),
 				os.path.basename( arnoldEnv.subst( "$INSTALL_LIB_NAME" ) ),
diff --git a/config/ie/options b/config/ie/options
@@ -431,28 +431,31 @@ DOXYGEN = os.path.join( "/software/apps/doxygen", os.environ["DOXYGEN_VERSION"],
 os.environ["OCIO"] = "/software/config/openColorIO/nuke-default/config.ocio"
 
 # import vars we need to get our doxygen and python wrappers working
-envVarsToImport = ["PATH",
-					"COMPILER",
-					"COMPILER_VERSION",
-					"PYTHONPATH",
-					"IEENV_ROOT",
-					"IEENV_WORKING_PATH",
-					"IEENV_LIBRARY_PREFIX_PATH",
-					"DOXYGEN_VERSION",
-					"IEENV_DEBUG",
-					"IEENV_DEBUG_PYTHON",
-					"IEENV_DEBUGGER",
-					"IEENV_DEBUGGER_ARGS",
-					"DELIGHT_CONF",
-					"SCONS_VERSION",
-					"DL_VERSION",
-					"DL_SHADERS_PATH",
-					"DL_DISPLAYS_PATH",
-					"solidangle_LICENSE",
-					"CORTEX_POINTDISTRIBUTION_TILESET",
-					"OCIO",
-					"IECORE_DEBUG_WAIT",
-					"CORTEX_PERFORMANCE_TEST"]
+envVarsToImport = [
+	"PATH",
+	"COMPILER",
+	"COMPILER_VERSION",
+	"PYTHONPATH",
+	"IEENV_ROOT",
+	"IEENV_WORKING_PATH",
+	"IEENV_LIBRARY_PREFIX_PATH",
+	"DOXYGEN_VERSION",
+	"IEENV_DEBUG",
+	"IEENV_DEBUG_PYTHON",
+	"IEENV_DEBUGGER",
+	"IEENV_DEBUGGER_ARGS",
+	"DELIGHT_CONF",
+	"SCONS_VERSION",
+	"DL_VERSION",
+	"DL_SHADERS_PATH",
+	"DL_DISPLAYS_PATH",
+	"solidangle_LICENSE",
+	"CORTEX_POINTDISTRIBUTION_TILESET",
+	"OCIO",
+	"IECORE_DEBUG_WAIT",
+	"CORTEX_PERFORMANCE_TEST",
+	"IECORE_RTLD_GLOBAL",
+]
 
 ENV_VARS_TO_IMPORT= " ".join(envVarsToImport)
 
diff --git a/python/IECore/__init__.py b/python/IECore/__init__.py
@@ -38,21 +38,27 @@
 #
 # Some parts of the IECore library are defined purely in Python. These are shown below.
 
-# Turn on RTLD_GLOBAL to avoid the dreaded cross module RTTI
-# errors on Linux. This causes libIECore etc to be loaded into
-# the global symbol table and those symbols to be shared between
-# modules. Without it, different python modules and/or libraries
-# can end up with their own copies of symbols, which breaks a
-# great many things.
-#
-# We use the awkward "__import__" approach to avoid importing sys
-# and ctypes into the IECore namespace.
-
-if __import__( "os" ).name == 'posix':
-	__import__( "sys" ).setdlopenflags(
-		__import__( "sys" ).getdlopenflags() | __import__( "ctypes" ).RTLD_GLOBAL
+import os, sys, ctypes
+if os.name == "posix" and os.environ.get( "IECORE_RTLD_GLOBAL", "1" ) == "1" :
+	# Historically, we had problems with cross-module RTTI on Linux, whereby
+	# different Python modules and/or libraries could end up with their own
+	# copies of symbols, which would break things like dynamic casts. We worked
+	# around this by using RTLD_GLOBAL so that everything was loaded into the
+	# global symbol table and shared, but this can cause hard-to-diagnose
+	# knock-on effects from unwanted sharing.
+	#
+	# We now manage symbol visibility properly so that RTTI symbols should not
+	# be duplicated between modules, and we intend to remove RTLD_GLOBAL. To aid
+	# the transition, this behaviour can be controlled by the
+	# `IECORE_RTLD_GLOBAL` environment variable, which currently defaults on.
+	## \todo Get everything tested, default to off, and then remove.
+	sys.setdlopenflags(
+		sys.getdlopenflags() | ctypes.RTLD_GLOBAL
 	)
 
+# Remove pollution of IECore namespace
+del os, sys, ctypes
+
 __import__( "imath" )
 
 from ._IECore import *
diff --git a/src/IECoreHoudini/plugin/Plugin.cpp b/src/IECoreHoudini/plugin/Plugin.cpp
@@ -72,7 +72,11 @@ extern "C"
 {
 	SYS_VISIBILITY_EXPORT void HoudiniDSOInit( UT_DSOInfo &dsoinfo )
 	{
-		dsoinfo.loadGlobal = true;
+		const char *forceGlobals = std::getenv( "IECORE_RTLD_GLOBAL" );
+		if( !forceGlobals || !strcmp( forceGlobals, "1" ) )
+		{
+			dsoinfo.loadGlobal = true;
+		}
 	}
 }
 
diff --git a/src/IECoreMaya/plugin/Loader.cpp b/src/IECoreMaya/plugin/Loader.cpp
@@ -66,7 +66,15 @@ IECORE_EXPORT MStatus initializePlugin( MObject obj )
 
 	std::string implName = pluginPath + "/impl/" + pluginName + ".so";
 
- 	g_libraryHandle = dlopen( implName.c_str(), RTLD_NOW | RTLD_GLOBAL );
+	const char *forceGlobals = std::getenv( "IECORE_RTLD_GLOBAL" );
+	if( !forceGlobals || !strcmp( forceGlobals, "1" ) )
+	{
+		g_libraryHandle = dlopen( implName.c_str(), RTLD_NOW | RTLD_GLOBAL );
+	}
+	else
+	{
+		g_libraryHandle = dlopen( implName.c_str(), RTLD_NOW );
+	}
 
 	if (! g_libraryHandle )
 	{

Original file line number	Diff line number	Diff line change
`@@ -72,7 +72,11 @@ extern "C"`
`72`	`72`	`{`
`73`	`73`	`SYS_VISIBILITY_EXPORT void HoudiniDSOInit( UT_DSOInfo &dsoinfo )`
`74`	`74`	`{`
`75`		`- dsoinfo.loadGlobal = true;`
	`75`	`+ const char *forceGlobals = std::getenv( "IECORE_RTLD_GLOBAL" );`
	`76`	`+ if( !forceGlobals \|\| !strcmp( forceGlobals, "1" ) )`
	`77`	`+ {`
	`78`	`+ dsoinfo.loadGlobal = true;`
	`79`	`+ }`
`76`	`80`	`}`
`77`	`81`	`}`
`78`	`82`