From f57607d2d58f6e7689a3550c84ba68ce42c6a7b3 Mon Sep 17 00:00:00 2001 From: Emiel Bruijntjes Date: Tue, 6 May 2014 09:34:12 +0200 Subject: When "apache reload" is called, the PHP-CPP library made the entire Apache process crash. This has been fixed --- include/extension.h | 16 ++++++++++++++++ include/namespace.h | 37 +++++++++++++++++++++++++++++++++++++ zend/extension.cpp | 12 ++++++++++++ zend/extensionimpl.cpp | 12 +++++++++++- zend/extensionimpl.h | 23 ++++++++++++++++++----- zend/namespace.cpp | 12 ++++++++++++ 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> _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 Namespace &add(Class &&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(new Class(std::move(type)))); @@ -96,6 +118,9 @@ public: template Namespace &add(const Class &type) { + // skip when locked + if (locked()) return *this; + // and add it to the list of classes _classes.push_back(std::unique_ptr(new Class(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(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(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(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(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 @@ -96,6 +96,18 @@ void *Extension::module() return _impl->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,15 +20,17 @@ 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 * @@ -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(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(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(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(name, function, arguments)); -- cgit v1.2.3