Fix for https://bugs.launchpad.net/moonshot/+bug/1705714 (Infinite loop in tids exit...

Refactored the C++ static object whose destructor was unloading the libraries.
The shibresolver library is now unloaded by a "smaller" static object that is
created after shibresolver is loaded. Creating that object late means that it
will be destroyed earlier, before the static objects in the log4shib library
are destroyed.

diff --git a/mech_eap/eap_mech.c b/mech_eap/eap_mech.c
index c88ecf6..403e2f8 100644 (file)
--- a/mech_eap/eap_mech.c
+++ b/mech_eap/eap_mech.c
@@ -205,6 +205,7 @@ gssEapInitiatorInit(OM_uint32 *minor)
 void
 gssEapFinalize(void)
 {
 void
 gssEapFinalize(void)
 {

+    wpa_printf(MSG_INFO, "### gssEapFinalize()");

     eap_peer_unregister_methods();
 }
 
     eap_peer_unregister_methods();
 }
 

diff --git a/mech_eap/util_attr.cpp b/mech_eap/util_attr.cpp
index beb283c..1931efe 100644 (file)
--- a/mech_eap/util_attr.cpp
+++ b/mech_eap/util_attr.cpp
@@ -64,9 +64,14 @@ GSSEAP_ONCE_CALLBACK(gssEapAttrProvidersInitInternal)
     gssEapLocalAttrProviderInit(&minor);
 #endif
 #ifdef HAVE_OPENSAML
     gssEapLocalAttrProviderInit(&minor);
 #endif
 #ifdef HAVE_OPENSAML

+    wpa_printf(MSG_INFO, "### gssEapAttrProvidersInitInternal(): Calling gssEapSamlAttrProvidersInit()");

     major = gssEapSamlAttrProvidersInit(&minor);
     major = gssEapSamlAttrProvidersInit(&minor);

-    if (GSS_ERROR(major))
+    if (GSS_ERROR(major)) {
+        wpa_printf(MSG_ERROR, "### gssEapAttrProvidersInitInternal(): Error returned from gssEapSamlAttrProvidersInit; major code is %08X; minor is %08X", major, minor);

         goto cleanup;
         goto cleanup;

+    }
+#else
+    wpa_printf(MSG_INFO, "### gssEapAttrProvidersInitInternal(): Don't have OpenSAML; not calling gssEapSamlAttrProvidersInit()");

 #endif
 
 cleanup:
 #endif
 
 cleanup:


@@ -74,6 +79,7 @@ cleanup:
     GSSEAP_ASSERT(major == GSS_S_COMPLETE);
 #endif
 
     GSSEAP_ASSERT(major == GSS_S_COMPLETE);
 #endif
 

+    wpa_printf(MSG_INFO, "### gssEapAttrProvidersInitInternal(): Setting gssEapAttrProvidersInitStatus to %08X", major);

     gssEapAttrProvidersInitStatus = major;
 
     GSSEAP_ONCE_LEAVE;
     gssEapAttrProvidersInitStatus = major;
 
     GSSEAP_ONCE_LEAVE;


@@ -90,16 +96,25 @@ gssEapAttrProvidersInit(OM_uint32 *minor)
     return gssEapAttrProvidersInitStatus;
 }
 
     return gssEapAttrProvidersInitStatus;
 }
 

+

 namespace {
 namespace {

+

     class finalize_class {
     public:
     class finalize_class {
     public:

+
+        finalize_class() {
+            wpa_printf(MSG_INFO, "### finalize_class::finalize_class(): Constructing");
+        }
+

       ~finalize_class()
            {
                OM_uint32 minor = 0;
       ~finalize_class()
            {
                OM_uint32 minor = 0;

+
+        wpa_printf(MSG_INFO, "### ~finalize_class::~finalize_class() : initStatus=%08x", gssEapAttrProvidersInitStatus);
+

                if (gssEapAttrProvidersInitStatus == GSS_S_COMPLETE) {
                if (gssEapAttrProvidersInitStatus == GSS_S_COMPLETE) {

-#ifdef HAVE_SHIBRESOLVER
-                   gssEapLocalAttrProviderFinalize(&minor);
-#endif
+            wpa_printf(MSG_INFO, "### ~finalize_class::~finalize_class() : really finalizing");
+

 #ifdef HAVE_OPENSAML
                    gssEapSamlAttrProvidersFinalize(&minor);
 #endif
 #ifdef HAVE_OPENSAML
                    gssEapSamlAttrProvidersFinalize(&minor);
 #endif

diff --git a/mech_eap/util_shib.cpp b/mech_eap/util_shib.cpp
index 7b62484..4ba70e0 100644 (file)
--- a/mech_eap/util_shib.cpp
+++ b/mech_eap/util_shib.cpp
@@ -78,6 +78,58 @@ using namespace opensaml;
 using namespace xercesc;
 #endif
 
 using namespace xercesc;
 #endif
 

+
+namespace {
+
+
+    class ShibFinalizer {
+    public:
+
+        static bool isShibInitialized() {return shibInitialized;}
+        static void createSingleton();
+
+    private:
+        ShibFinalizer(): is_extra(false) {
+            if (shibInitialized) {
+                // This should never, ever happen. Initialization is (supposed to be)
+                // funneled through a single thread, so there should be no race
+                // conditions here. And only this class sets this flag, and there's
+                // only a single instance of this class.
+                wpa_printf(MSG_ERROR, "### ShibFinalizer::ShibFinalizer(): Attempt to construct an extraneous instance!");
+                is_extra = true;
+            }
+            else {
+                wpa_printf(MSG_INFO, "### ShibFinalizer::ShibFinalizer(): Constructing");
+                shibInitialized = true;
+            }
+        }
+
+        ~ShibFinalizer() {
+            if (!is_extra) {
+                wpa_printf(MSG_INFO, "### ShibFinalizer::~ShibFinalizer(): Destructing");
+                gss_eap_shib_attr_provider::finalize();
+                shibInitialized = false;
+            }
+            else {
+                wpa_printf(MSG_INFO, "### ShibFinalizer::~ShibFinalizer(): This was an extraneous instance; not destructing anything.");
+            }
+        }
+
+        bool is_extra;
+        static bool shibInitialized;
+    };
+}
+
+
+bool ShibFinalizer::shibInitialized = false;
+
+void ShibFinalizer::createSingleton() {
+    // This object's constructor is invoked on the first call to this method.
+    // At exit, its destructor will terminate Shibboleth.
+    static ShibFinalizer finalizer;
+}
+
+

 gss_eap_shib_attr_provider::gss_eap_shib_attr_provider(void)
 {
     m_initialized = false;
 gss_eap_shib_attr_provider::gss_eap_shib_attr_provider(void)
 {
     m_initialized = false;


@@ -463,13 +515,22 @@ gss_eap_shib_attr_provider::init(void)
 {
     bool ret = false;
 
 {
     bool ret = false;
 

+    if (ShibFinalizer::isShibInitialized()) {
+        wpa_printf(MSG_INFO, "### gss_eap_shib_attr_provider::init(): ShibResolver library is already initialized; ignoring.");
+        return true;
+    }
+
+    wpa_printf(MSG_INFO, "### gss_eap_shib_attr_provider::init(): Initializing ShibResolver library");
+

     try {
         ret = ShibbolethResolver::init();
     } catch (exception &e) {
     }
 
     try {
         ret = ShibbolethResolver::init();
     } catch (exception &e) {
     }
 

-    if (ret)
+    if (ret) {
+        ShibFinalizer::createSingleton();

         gss_eap_attr_ctx::registerProvider(ATTR_TYPE_LOCAL, createAttrContext);
         gss_eap_attr_ctx::registerProvider(ATTR_TYPE_LOCAL, createAttrContext);

+    }

 
     return ret;
 }
 
     return ret;
 }


@@ -477,6 +538,7 @@ gss_eap_shib_attr_provider::init(void)
 void
 gss_eap_shib_attr_provider::finalize(void)
 {
 void
 gss_eap_shib_attr_provider::finalize(void)
 {

+    wpa_printf(MSG_INFO, "### gss_eap_shib_attr_provider::finalize(): calling ShibbolethResolver::term()");

     gss_eap_attr_ctx::unregisterProvider(ATTR_TYPE_LOCAL);
     ShibbolethResolver::term();
 }
     gss_eap_attr_ctx::unregisterProvider(ATTR_TYPE_LOCAL);
     ShibbolethResolver::term();
 }


@@ -534,6 +596,7 @@ gss_eap_shib_attr_provider::duplicateAttributes(const vector <Attribute *>src)
     return dst;
 }
 
     return dst;
 }
 

+

 OM_uint32
 gssEapLocalAttrProviderInit(OM_uint32 *minor)
 {
 OM_uint32
 gssEapLocalAttrProviderInit(OM_uint32 *minor)
 {


@@ -541,14 +604,6 @@ gssEapLocalAttrProviderInit(OM_uint32 *minor)
         *minor = GSSEAP_SHIB_INIT_FAILURE;
         return GSS_S_FAILURE;
     }
         *minor = GSSEAP_SHIB_INIT_FAILURE;
         return GSS_S_FAILURE;
     }

-    return GSS_S_COMPLETE;
-}
-
-OM_uint32
-gssEapLocalAttrProviderFinalize(OM_uint32 *minor)
-{
-    gss_eap_shib_attr_provider::finalize();

 
 

-    *minor = 0;

     return GSS_S_COMPLETE;
 }
     return GSS_S_COMPLETE;
 }