When "apache reload" is called, the PHP-CPP library made the entire Apache process crash. This has been fixed

6 files changed, 106 insertions, 6 deletions

diff --git a/include/extension.h b/include/extension.h
index f03c8fb..562fceb 100644
--- a/include/extension.h
+++ b/include/extension.h
@@ -111,6 +111,9 @@ public:
      */
     Extension &add(Ini &&ini)
     {
+        // skip when locked
+        if (locked()) return *this;
+        
         // and add it to the list of classes
         _ini_entries.emplace_back(new Ini(std::move(ini)));
         
@@ -125,6 +128,9 @@ public:
      */
     Extension &add(const Ini &ini)
     {
+        // skip when locked
+        if (locked()) return *this;
+
         // and add it to the list of classes
         _ini_entries.emplace_back(new Ini(ini));
         
@@ -180,6 +186,16 @@ public:
     {
         return module();
     }
+    
+protected:
+    /**
+     *  Is the extension object in a locked state? This happens after the
+     *  get_module() function was called for the first time ("apache reload"
+     *  forces a new call to get_module())
+     * 
+     *  @return bool
+     */
+    virtual bool locked() const override;
 
 private:
     /**
diff --git a/include/namespace.h b/include/namespace.h
index 828c12c..e713850 100644
--- a/include/namespace.h
+++ b/include/namespace.h
@@ -48,6 +48,25 @@ protected:
      */
     std::list<std::shared_ptr<Namespace>> _namespaces;
 
+    /**
+     *  Is the object locked?
+     * 
+     *  After the object is locked, no more elements can be added to it.
+     *  This happens after the call to get_module - it the no longer makes
+     *  sense to add more objects. When 'apache reload' is executed, the
+     *  get_module() function is called for a second (or third, or fourth)
+     *  time, but the classes, functions and namespaces will then not be
+     *  filled.
+     * 
+     *  @var bool
+     */
+    virtual bool locked() const
+    {
+        // by default, namespaces are not locked (only derived extension 
+        // objects can end up in a locked state
+        return false;
+    }
+
 
 public:
     /**
@@ -81,6 +100,9 @@ public:
     template<typename T>
     Namespace &add(Class<T> &&type)
     {
+        // skip when locked
+        if (locked()) return *this;
+        
         // make a copy of the object, and add it to the list of classes
         _classes.push_back(std::unique_ptr<ClassBase>(new Class<T>(std::move(type))));
         
@@ -96,6 +118,9 @@ public:
     template<typename T>
     Namespace &add(const Class<T> &type)
     {
+        // skip when locked
+        if (locked()) return *this;
+
         // and add it to the list of classes
         _classes.push_back(std::unique_ptr<ClassBase>(new Class<T>(type)));
         
@@ -110,6 +135,9 @@ public:
      */
     Namespace &add(Interface &&interface)
     {
+        // skip when locked
+        if (locked()) return *this;
+
         // make a copy and add it to the list of classes
         _classes.push_back(std::unique_ptr<ClassBase>(new Interface(std::move(interface))));
         
@@ -124,6 +152,9 @@ public:
      */
     Namespace &add(const Interface &interface)
     {
+        // skip when locked
+        if (locked()) return *this;
+
         // make a copy and add it to the list of classes
         _classes.push_back(std::unique_ptr<ClassBase>(new Interface(interface)));
         
@@ -138,6 +169,9 @@ public:
      */
     Namespace &add(Namespace &&ns)
     {
+        // skip when locked
+        if (locked()) return *this;
+
         // add it to the list of namespaces
         _namespaces.push_back(std::unique_ptr<Namespace>(new Namespace(std::move(ns))));
         
@@ -152,6 +186,9 @@ public:
      */
     Namespace &add(const Namespace &ns)
     {
+        // skip when locked
+        if (locked()) return *this;
+
         // make a copy and add it to the list of namespaces
         _namespaces.push_back(std::unique_ptr<Namespace>(new Namespace(ns)));
         
diff --git a/zend/extension.cpp b/zend/extension.cpp
index 9685b32..1545f89 100644
--- a/zend/extension.cpp
+++ b/zend/extension.cpp
@@ -97,6 +97,18 @@ void *Extension::module()
 }
 
 /**
+ *  Is the extension object in a locked state? This happens after the
+ *  get_module() function was called for the first time ("apache reload"
+ *  forces a new call to get_module())
+ * 
+ *  @return bool
+ */
+bool Extension::locked() const
+{
+    return _impl->locked();
+}
+
+/**
  *  End of namespace
  */
 }
diff --git a/zend/extensionimpl.cpp b/zend/extensionimpl.cpp
index ea0c049..f073acf 100644
--- a/zend/extensionimpl.cpp
+++ b/zend/extensionimpl.cpp
@@ -142,6 +142,10 @@ int ExtensionImpl::processStartup(int type, int module_number TSRMLS_DC)
     // initialize the extension
     extension->initialize(TSRMLS_C);
     
+    // remember that we're initialized (when you use "apache reload" it is 
+    // possible that the processStartup() method is called more than once)
+    extension->_locked = true;
+    
     // is the callback registered?
     if (extension->_onStartup) extension->_onStartup();
 
@@ -164,6 +168,12 @@ int ExtensionImpl::processShutdown(int type, int module_number TSRMLS_DC)
     // unregister the ini entries
     zend_unregister_ini_entries(module_number TSRMLS_CC);
 
+    // destruct the ini entries
+    if (extension->_ini) delete[] extension->_ini;
+
+    // forget the ini entries
+    extension->_ini = nullptr;
+
     // is the callback registered?
     if (extension->_onShutdown) extension->_onShutdown();
     
@@ -272,7 +282,7 @@ ExtensionImpl::~ExtensionImpl()
  */
 zend_module_entry *ExtensionImpl::module()
 {
-    // check if functions we're already defined
+    // check if functions were already defined
     if (_entry.functions) return &_entry;
 
     // the number of functions
diff --git a/zend/extensionimpl.h b/zend/extensionimpl.h
index ba3e6c8..0e9a289 100644
--- a/zend/extensionimpl.h
+++ b/zend/extensionimpl.h
@@ -20,16 +20,18 @@ class ExtensionImpl : public ExtensionBase
 protected:
     /**
      *  The information that is passed to the Zend engine
-     * 
-     *  Although it would be slightly faster to not make this a pointer, this
-     *  would require that client code also includes the PHP header files, which
-     *  we try to prevent with the PHP-CPP library, so we allocate it dynamically.
-     * 
      *  @var zend_module_entry
      */
     zend_module_entry _entry;
     
     /**
+     *  Is the object locked? This prevents crashes for 'apache reload'
+     *  because we then do not have to re-initialize the entire php engine
+     *  @var bool
+     */
+    bool _locked = false;
+    
+    /**
      *  The .ini entries
      *  
      *  @var zend_ini_entry
@@ -56,6 +58,17 @@ public:
      */
     virtual ~ExtensionImpl();
     
+    /** 
+     *  Is the object locked (true) or is it still possible to add more functions,
+     *  classes and other elements to it?
+     *  @return bool
+     */
+    bool locked()
+    {
+        // return member
+        return _locked;
+    }
+    
     /**
      *  Retrieve the module entry
      * 
diff --git a/zend/namespace.cpp b/zend/namespace.cpp
index 2b4b62a..9274bf0 100644
--- a/zend/namespace.cpp
+++ b/zend/namespace.cpp
@@ -22,6 +22,9 @@ namespace Php {
  */
 Namespace &Namespace::add(const char *name, const native_callback_0 &function, const Arguments &arguments)
 {
+    // skip when locked
+    if (locked()) return *this;
+
     // add a function
     _functions.push_back(std::make_shared<Function>(name, function, arguments));
     
@@ -38,6 +41,9 @@ Namespace &Namespace::add(const char *name, const native_callback_0 &function, c
  */
 Namespace &Namespace::add(const char *name, const native_callback_1 &function, const Arguments &arguments)
 {
+    // skip when locked
+    if (locked()) return *this;
+
     // add a function
     _functions.push_back(std::make_shared<Function>(name, function, arguments));
 
@@ -54,6 +60,9 @@ Namespace &Namespace::add(const char *name, const native_callback_1 &function, c
  */
 Namespace &Namespace::add(const char *name, const native_callback_2 &function, const Arguments &arguments)
 {
+    // skip when locked
+    if (locked()) return *this;
+
     // add a function
     _functions.push_back(std::make_shared<Function>(name, function, arguments));
 
@@ -70,6 +79,9 @@ Namespace &Namespace::add(const char *name, const native_callback_2 &function, c
  */
 Namespace &Namespace::add(const char *name, const native_callback_3 &function, const Arguments &arguments)
 {
+    // skip when locked
+    if (locked()) return *this;
+
     // add a function
     _functions.push_back(std::make_shared<Function>(name, function, arguments));