poco icon indicating copy to clipboard operation
poco copied to clipboard

System exception: "cannot lock mutex" when using Util :: XMLConfiguration/AbstractConfiguration --> createView() -> keys()

Open DavHee opened this issue 1 year ago • 4 comments

Describe the bug My application encounters a Poco SystemException when accessing the keys of a xml sub-view.

To Reproduce Mini example "poco_bug.cpp":

#include<Poco/Util/AbstractConfiguration.h>
#include<Poco/Util/XMLConfiguration.h>
#include<iostream>
using namespace Poco::Util;

int main()
{
        std::vector<std::string> confKeys;
        AbstractConfiguration* cfg = new XMLConfiguration("conf.xml");
        Poco::Util::AbstractConfiguration* sysView = cfg->createView("prop3");
        try {
                sysView->keys(confKeys);
        } catch (const Poco::SystemException& e) {
                std::cerr << "SystemException: " << e.displayText() << std::endl;
                std::cerr << "Code: " << e.code() << std::endl;
        }
        return 0;
}

The try-catch is only used to receive further information.

Read xml "conf.xml":

<config>
    <prop1>value1</prop1>
    <prop2>value2</prop2>
    <prop3>
        <prop4 attr="value3"/>
        <prop4 attr="value4"/>
    </prop3>
    <prop5 id="first">value5</prop5>
    <prop5 id="second">value6</prop5>
</config>

Compiled and executed with:

g++ poco_bug.cpp -lPocoFoundation -lPocoXML -lPocoUtil -g
./a.out

Expected behavior Read keys without throwing system exception.

Logs Output of the mini example:

SystemException: System exception: cannot lock mutex
Code: 0

Output of valgrind:

# valgrind ./a.out
==4743== Memcheck, a memory error detector
==4743== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==4743== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
==4743== Command: ./a.out
==4743==
==4743== Invalid read of size 4
==4743==    at 0x4DC9FE4: pthread_mutex_lock@@GLIBC_2.2.5 (pthread_mutex_lock.c:80)
==4743==    by 0x4A43120: ??? (in /usr/lib/x86_64-linux-gnu/libPocoUtil.so.80)
==4743==    by 0x4A4720D: Poco::Util::AbstractConfiguration::keys(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >&) const (in /usr/lib/x86_64-linux-gnu/libPocoUtil.so.80)
==4743==    by 0x10A5A4: main (poco_bug.cpp:12)
==4743==  Address 0x51e2bd8 is 488 bytes inside a block of size 552 free'd
==4743==    at 0x484A61D: operator delete(void*, unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4743==    by 0x10A840: Poco::RefCountedObject::release() const (RefCountedObject.h:82)
==4743==    by 0x10AB16: Poco::AutoPtr<Poco::Util::AbstractConfiguration>::~AutoPtr() (AutoPtr.h:96)
==4743==    by 0x10A572: main (poco_bug.cpp:10)
==4743==  Block was alloc'd at
==4743==    at 0x4846FA3: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4743==    by 0x4A4B934: Poco::Util::AbstractConfiguration::createView(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (in /usr/lib/x86_64-linux-gnu/libPocoUtil.so.80)
==4743==    by 0x10A54D: main (poco_bug.cpp:10)
==4743==
==4743== Invalid read of size 4
==4743==    at 0x4DC992C: __pthread_mutex_lock_full (pthread_mutex_lock.c:198)
==4743==    by 0x4A43120: ??? (in /usr/lib/x86_64-linux-gnu/libPocoUtil.so.80)
==4743==    by 0x4A4720D: Poco::Util::AbstractConfiguration::keys(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >&) const (in /usr/lib/x86_64-linux-gnu/libPocoUtil.so.80)
==4743==    by 0x10A5A4: main (poco_bug.cpp:12)
==4743==  Address 0x51e2bd8 is 488 bytes inside a block of size 552 free'd
==4743==    at 0x484A61D: operator delete(void*, unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4743==    by 0x10A840: Poco::RefCountedObject::release() const (RefCountedObject.h:82)
==4743==    by 0x10AB16: Poco::AutoPtr<Poco::Util::AbstractConfiguration>::~AutoPtr() (AutoPtr.h:96)
==4743==    by 0x10A572: main (poco_bug.cpp:10)
==4743==  Block was alloc'd at
==4743==    at 0x4846FA3: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4743==    by 0x4A4B934: Poco::Util::AbstractConfiguration::createView(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (in /usr/lib/x86_64-linux-gnu/libPocoUtil.so.80)
==4743==    by 0x10A54D: main (poco_bug.cpp:10)
==4743==
SystemException: System exception: cannot lock mutex
Code: 0
==4743==
==4743== HEAP SUMMARY:
==4743==     in use at exit: 54,904 bytes in 20 blocks
==4743==   total heap usage: 158 allocs, 138 frees, 177,095 bytes allocated
==4743==
==4743== LEAK SUMMARY:
==4743==    definitely lost: 536 bytes in 1 blocks
==4743==    indirectly lost: 54,368 bytes in 19 blocks
==4743==      possibly lost: 0 bytes in 0 blocks
==4743==    still reachable: 0 bytes in 0 blocks
==4743==         suppressed: 0 bytes in 0 blocks
==4743== Rerun with --leak-check=full to see details of leaked memory
==4743==
==4743== For lists of detected and suppressed errors, rerun with: -s
==4743== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)

Please add relevant environment information:

  • Docker Image ubuntu:24.04
  • apt update; apt install g++ libpoco-dev
    • (libpoco-dev: Installed: 1.11.0-4.1build2 ; g++: Installed: 4:13.2.0-7ubuntu1)

With poco version 1.9.4 this was working. I also tried newer versions (e.g. 1.12.0, 1.13.3, 1.14.0) and also encountered this problem.

DavHee avatar Jan 10 '25 14:01 DavHee

At https://github.com/b1-systems we discovered a workaround for this.

diff against main branch

The workaround basically reverts the change from AbstractConfiguration * to AbstractConfiguration::Ptr, which is sufficient to eliminate the bug (crash) and the valgrind memory check error iff rebuilding every binary against the patched poco lib (this changes the API and ABI of the library and therefore is not compatible).

The problem is a use-after-free of the mutex after the automagic pointer has freed the object already. (This might need a CVE.)

The patch is likely not acceptable upstream, which is why we’re not entering it as a pull request at this point. Upstream will instead want to identify why the refcount goes down to zero if the object is still in use and add an appropriate refcount increment at the correct place, but I’m a C programmer, not a C++ programmer, so I’ll leave that to upstream, meanwhile the workaround is there for those who need it.

mirabilos avatar Oct 21 '25 15:10 mirabilos

TLDR: This is not a bug in the Poco library, but rather invalid usage of the library!


Hi,

we identified the root cause of that issue. The reason it's no longer working is the switch to smart pointers (from https://github.com/pocoproject/poco/commit/1bf40a0cd28be83e6243a0524be19118792c8cd1). The target code snippet was casting the smart pointer back to a raw pointer, which is no longer valid.

The code snippet requires the following update:

#include<Poco/Util/AbstractConfiguration.h>
#include<Poco/Util/XMLConfiguration.h>
#include<iostream>
using namespace Poco::Util;

int main()
{
        std::vector<std::string> confKeys;
        AbstractConfiguration* cfg = new XMLConfiguration("conf.xml");
        AbstractConfiguration::Ptr sysView = cfg->createView("prop3");
        try {
                sysView->keys(confKeys);
        } catch (const Poco::SystemException& e) {
                std::cerr << "SystemException: " << e.displayText() << std::endl;
                std::cerr << "Code: " << e.code() << std::endl;
        }
        return 0;
}

You can also use the auto keyword instead of explicitly declaring AbstractConfiguration::Ptr: auto sysView = cfg->createView("prop3");.

SuperNascher avatar Oct 23 '25 12:10 SuperNascher

On Thu, 23 Oct 2025, Kevin R. wrote:

The code snippet requires the following update:

As diff, that reads:

@@ -8,3 +8,3 @@ std::vectorstd::string confKeys; AbstractConfiguration* cfg = new XMLConfiguration("conf.xml");

  •    Poco::Util::AbstractConfiguration* sysView = cfg->createView("prop3");
    
  •    AbstractConfiguration::Ptr sysView = cfg->createView("prop3");
    

TLDR: This is not a bug in the Poco library, but rather invalid usage of the library!

Do you not have a stable API (looking at the Debian packaging, you seem to not have a stable ABI, but the API at least…)?

And even if not, this is bad style. It would have been better to make code not even compile against the new version if you break it like this.

I’m only an intermediate, but I’ll see whether the party having the problem can get the code updated.

mirabilos avatar Oct 24 '25 02:10 mirabilos

On Thu, 23 Oct 2025, Kevin R. wrote: The code snippet requires the following update: As diff, that reads:

@@ -8,3 +8,3 @@ std::vectorstd::string confKeys; AbstractConfiguration* cfg = new XMLConfiguration("conf.xml");

  •    Poco::Util::AbstractConfiguration* sysView = cfg->createView("prop3");
    
  •    AbstractConfiguration::Ptr sysView = cfg->createView("prop3");
    

TLDR: This is not a bug in the Poco library, but rather invalid usage of the library! Do you not have a stable API (looking at the Debian packaging, you seem to not have a stable ABI, but the API at least…)?

And even if not, this is bad style. It would have been better to make code not even compile against the new version if you break it like this.

I’m only an intermediate, but I’ll see whether the party having the problem can get the code updated.

I'm not an upstream developer or else!

SuperNascher avatar Oct 24 '25 08:10 SuperNascher

Resolution

This is not a bug in POCO, but a consequence of an API change in POCO 1.11 (commit 1bf40a0c) where createView() and createLocalView() were changed to return AbstractConfiguration::Ptr (AutoPtr) instead of raw pointers.

The Problem

The user code:

AbstractConfiguration* sysView = cfg->createView("prop3");  // WRONG!
sysView->keys(confKeys);  // Use-after-free!

What happens:

  1. createView() returns AutoPtr<AbstractConfiguration> with refcount=1
  2. AutoPtr::operator T*() implicitly converts to raw pointer
  3. Temporary AutoPtr destroyed → refcount=0 → object deleted
  4. sysView points to freed memory → crash

Correct Usage

AbstractConfiguration::Ptr sysView = cfg->createView("prop3");
// or
auto sysView = cfg->createView("prop3");

Improvements Made

  1. PR #5119: Added [[nodiscard]] attribute and documentation to createView()/createLocalView() methods
  2. Migration note: Added to CHANGELOG.md (branch 4383-rename-changelog) warning about this API change

While this doesn't prevent the problematic code from compiling (due to AutoPtr's implicit conversion operator), the documentation now explicitly warns about this pitfall.

matejk avatar Dec 17 '25 22:12 matejk