fix(PropertyFileConfiguration): Circular reference in Application::lo…

CHANGELOG

@@ -1,6 +1,25 @@
 This is the changelog file for the POCO C++ Libraries.
 
 
+Release 1.15.3 (Unreleased)
+===========================
+
+API Changes:
+
+- GH #5322 PropertyFileConfiguration: the optional parent-configuration
+  parameter (added in 1.15.1 via #5253) is now an AbstractConfiguration*
+  raw non-owning pointer instead of AbstractConfiguration::Ptr. Callers
+  must keep the parent alive for the child's lifetime. This fixes a
+  circular reference between LayeredConfiguration and child
+  PropertyFileConfiguration instances in Application::loadConfiguration().
+  The Ptr-taking overloads are retained as deprecated forwarders to
+  surface the lifetime contract at compile time; they will be removed
+  in a future release. Passing a temporary Ptr would leave _pParentConfig
+  dangling and should be replaced with a named variable whose lifetime
+  outlives the child. All three PropertyFileConfiguration constructors
+  are now explicit to prevent accidental implicit conversions.
+
+
 Release 1.15.2 (2026-04-16)
 ===========================
 

Util/include/Poco/Util/PropertyFileConfiguration.h

@@ -66,22 +66,41 @@ class Util_API PropertyFileConfiguration: public MapConfiguration
 	/// a colon ':' nor an equal sign '=' character.
 {
 public:
-	PropertyFileConfiguration(AbstractConfiguration::Ptr pParentConfig = nullptr);
+	explicit PropertyFileConfiguration(AbstractConfiguration* pParentConfig = nullptr);
 		/// Creates an empty PropertyFileConfiguration.
 		/// If pParentConfig is not null, it is used to expand ${variable}
 		/// references in !include directive paths.
+		/// The parent configuration is not owned by this object;
+		/// the caller must ensure it outlives this configuration.
 
-	PropertyFileConfiguration(std::istream& istr, AbstractConfiguration::Ptr pParentConfig = nullptr);
+	explicit PropertyFileConfiguration(std::istream& istr, AbstractConfiguration* pParentConfig = nullptr);
 		/// Creates an PropertyFileConfiguration and loads the configuration data
 		/// from the given stream, which must be in properties file format.
 		/// If pParentConfig is not null, it is used to expand ${variable}
 		/// references in !include directive paths.
+		/// The parent configuration is not owned by this object;
+		/// the caller must ensure it outlives this configuration.
 
-	PropertyFileConfiguration(const std::string& path, AbstractConfiguration::Ptr pParentConfig = nullptr);
+	explicit PropertyFileConfiguration(const std::string& path, AbstractConfiguration* pParentConfig = nullptr);
 		/// Creates an PropertyFileConfiguration and loads the configuration data
 		/// from the given file, which must be in properties file format.
 		/// If pParentConfig is not null, it is used to expand ${variable}
 		/// references in !include directive paths.
+		/// The parent configuration is not owned by this object;
+		/// the caller must ensure it outlives this configuration.
+
+	POCO_DEPRECATED("Pass a raw AbstractConfiguration*; PropertyFileConfiguration does not take ownership of the parent")
+	explicit PropertyFileConfiguration(AbstractConfiguration::Ptr pParentConfig);
+		/// Deprecated. Use the AbstractConfiguration* overload instead.
+		/// Forwards to the raw-pointer constructor.
+
+	POCO_DEPRECATED("Pass a raw AbstractConfiguration*; PropertyFileConfiguration does not take ownership of the parent")
+	PropertyFileConfiguration(std::istream& istr, AbstractConfiguration::Ptr pParentConfig);
+		/// Deprecated. Use the AbstractConfiguration* overload instead.
+
+	POCO_DEPRECATED("Pass a raw AbstractConfiguration*; PropertyFileConfiguration does not take ownership of the parent")
+	PropertyFileConfiguration(const std::string& path, AbstractConfiguration::Ptr pParentConfig);
+		/// Deprecated. Use the AbstractConfiguration* overload instead.
 
 	void load(std::istream& istr);
 		/// Loads the configuration data from the given stream, which
@@ -173,7 +192,12 @@ class Util_API PropertyFileConfiguration: public MapConfiguration
 	static int readChar(std::istream& istr);
 	static std::string escapeValue(const std::string& value);
 
-	AbstractConfiguration::Ptr _pParentConfig;
+	AbstractConfiguration* _pParentConfig = nullptr;
+		/// Non-owning back-pointer to the parent configuration used for
+		/// ${variable} expansion in !include directive paths.
+		/// This is intentionally a raw pointer to avoid a circular reference:
+		/// the parent (typically LayeredConfiguration) owns this
+		/// PropertyFileConfiguration, so the parent always outlives the child.
 	std::map<std::string, std::string> _sourceMap;
 	std::string _rootFile;
 };

Util/src/Application.cpp

@@ -203,7 +203,7 @@ int Application::loadConfiguration(int priority)
 	Path confPath;
 	if (findAppConfigFile(appPath.getBaseName(), "properties"s, confPath))
 	{
-		_pConfig->add(new PropertyFileConfiguration(confPath.toString(), AbstractConfiguration::Ptr(_pConfig, true)), priority, false);
+		_pConfig->add(new PropertyFileConfiguration(confPath.toString(), _pConfig.get()), priority, false);
 		++n;
 	}
 #ifndef POCO_UTIL_NO_INIFILECONFIGURATION
@@ -245,7 +245,7 @@ void Application::loadConfiguration(const std::string& path, int priority)
 	std::string ext = confPath.getExtension();
 	if (icompare(ext, "properties") == 0)
 	{
-		_pConfig->add(new PropertyFileConfiguration(confPath.toString(), AbstractConfiguration::Ptr(_pConfig, true)), priority, false);
+		_pConfig->add(new PropertyFileConfiguration(confPath.toString(), _pConfig.get()), priority, false);
 		++n;
 	}
 #ifndef POCO_UTIL_NO_INIFILECONFIGURATION

Util/src/PropertyFileConfiguration.cpp

@@ -34,23 +34,41 @@ namespace Poco::Util {
 PropertyFileConfiguration::~PropertyFileConfiguration() = default;
 
 
+PropertyFileConfiguration::PropertyFileConfiguration(AbstractConfiguration* pParentConfig):
+	_pParentConfig(pParentConfig)
+{
+}
+
+
+PropertyFileConfiguration::PropertyFileConfiguration(std::istream& istr, AbstractConfiguration* pParentConfig):
+	_pParentConfig(pParentConfig)
+{
+	load(istr);
+}
+
+
+PropertyFileConfiguration::PropertyFileConfiguration(const std::string& path, AbstractConfiguration* pParentConfig):
+	_pParentConfig(pParentConfig)
+{
+	load(path);
+}
+
+
 PropertyFileConfiguration::PropertyFileConfiguration(AbstractConfiguration::Ptr pParentConfig):
-	_pParentConfig(std::move(pParentConfig))
+	PropertyFileConfiguration(pParentConfig.get())
 {
 }
 
 
 PropertyFileConfiguration::PropertyFileConfiguration(std::istream& istr, AbstractConfiguration::Ptr pParentConfig):
-	_pParentConfig(std::move(pParentConfig))
+	PropertyFileConfiguration(istr, pParentConfig.get())
 {
-	load(istr);
 }
 
 
 PropertyFileConfiguration::PropertyFileConfiguration(const std::string& path, AbstractConfiguration::Ptr pParentConfig):
-	_pParentConfig(std::move(pParentConfig))
+	PropertyFileConfiguration(path, pParentConfig.get())
 {
-	load(path);
 }
 
 

Util/testsuite/src/PropertyFileConfigurationTest.cpp

@@ -339,7 +339,7 @@ void PropertyFileConfigurationTest::testInclude()
 		ostr << "!include ${extDir}" << includedParentName << "\n";
 	}
 
-	AutoPtr<PropertyFileConfiguration> pConfParent = new PropertyFileConfiguration(mainFileParent.path(), pParent);
+	AutoPtr<PropertyFileConfiguration> pConfParent = new PropertyFileConfiguration(mainFileParent.path(), pParent.get());
 	assertTrue (pConfParent->getString("main.prop") == "mainValue");
 	assertTrue (pConfParent->getString("parent.prop") == "parentValue");
 }
@@ -812,6 +812,23 @@ void PropertyFileConfigurationTest::testIncludeManagement()
 }
 
 
+void PropertyFileConfigurationTest::testNoCircularReference()
+{
+	using Poco::Util::MapConfiguration;
+
+	AutoPtr<MapConfiguration> pParent = new MapConfiguration;
+	pParent->setString("base.dir", "/tmp/poco-test");
+	assertTrue (pParent->referenceCount() == 1);
+
+	{
+		AutoPtr<PropertyFileConfiguration> pChild = new PropertyFileConfiguration(pParent.get());
+		assertTrue (pParent->referenceCount() == 1);
+		(void) pChild;
+	}
+	assertTrue (pParent->referenceCount() == 1);
+}
+
+
 AbstractConfiguration::Ptr PropertyFileConfigurationTest::allocConfiguration() const
 {
 	return new PropertyFileConfiguration;
@@ -841,6 +858,7 @@ CppUnit::Test* PropertyFileConfigurationTest::suite()
 	CppUnit_addTest(pSuite, PropertyFileConfigurationTest, testClearResetsProvenance);
 	CppUnit_addTest(pSuite, PropertyFileConfigurationTest, testGetSourceFilesCoversAllKeys);
 	CppUnit_addTest(pSuite, PropertyFileConfigurationTest, testIncludeManagement);
+	CppUnit_addTest(pSuite, PropertyFileConfigurationTest, testNoCircularReference);
 
 	return pSuite;
 }

Util/testsuite/src/PropertyFileConfigurationTest.h

@@ -32,6 +32,7 @@ class PropertyFileConfigurationTest: public AbstractConfigurationTest
 	void testClearResetsProvenance();
 	void testGetSourceFilesCoversAllKeys();
 	void testIncludeManagement();
+	void testNoCircularReference();
 
 	void setUp();
 	void tearDown();