Check _ptr != ptr is not enough in AutoPtr.assign

[funny-falcon]

poco/Foundation/include/Poco/AutoPtr.h

Lines 122 to 127 in 3838070

	if (&ptr != this)
	{
	if (_ptr) _ptr->release();
	_ptr = ptr._ptr;
	if (_ptr) _ptr->duplicate();
	}

New object could be one referenced by old with it's internal AutoPtr.
If we release old pointer before duplicate new we could have dangled pointer problem already.

[aleks-f]

@funny-falcon how does the dangling pointer happen exactly? the check is whether the assigned-from AutoPtr is the same as the assigned-to one. If they are different AutoPtrs pointing to the same object, assignment can be skipped because they are both in the desired state already, but there is no harm done even if assignment happens - the assigned-to release() will not delete because assigned-from still keeps the reference count > 0

[funny-falcon]

Using poco 1.11.0 from Ubuntu 23.04

#include <iostream>
#include <string>
#include <Poco/RefCountedObject.h>
#include <Poco/AutoPtr.h>

class RCO : public Poco::RefCountedObject
{
    public:
        RCO(int _i) : i(_i), name("RCO(" + std::to_string(_i) + ")")
        {
            std::cout << name << " constructed\n";
        }

        int i;
        std::string name;
        Poco::AutoPtr<RCO> linked;

        ~RCO()
        {
            std::cout << name << " destructed\n";
            name = "BOOMED " + name;
        }
};

Poco::AutoPtr<RCO>
make_two()
{
    Poco::AutoPtr<RCO> a(new RCO(1));
    a->linked = new RCO(2);
    return a;
}

int
main() {
    Poco::AutoPtr<RCO> a;

    a = make_two();
    std::cout << "will it boom?\n";
    a = a->linked;

    std::cout << "hello " << a->name << "\n";
}

Output:

~/tmp/poco_issue3979 $ g++ issue3979.cc -lPocoFoundation
~/tmp/poco_issue3979 $ ./a.out 
RCO(1) constructed
RCO(2) constructed
will it boom?
RCO(1) destructed
RCO(2) destructed
hello BOOMED RCO(2)

[funny-falcon]

Is it desired behavior?

[aleks-f]

Is it desired behavior?

I'd say, it is not a desirable use - you are assigning a member AutoPtr to the owner object, which is also an AutoPtr, so the operation causes the member to inadvertently be destructed during the assignment.
Sadly, we don't provide a shield against shooting yourself in the foot.

You can do this instead and everything will be fine:

auto b = a->linked;
a = b;