View Issue Details

IDProjectCategoryView StatusLast Update
0002030OpenFOAMBugpublic2016-03-20 10:36
Reporterprojectionist Assigned Tohenry  
PrioritynormalSeveritytrivialReproducibilityalways
Status resolvedResolutionfixed 
PlatformLinuxOSUbuntuOS Version14.04
Product Versiondev 
Fixed in Versiondev 
Summary0002030: DebugSwitch objectRegistry leads to segfault at destruction of Time object when the application terminates
DescriptionThis is a rather academic bug report. Not only does the bug only appear when activating the debugging switch, it was also found in my former office at university by a colleague.

When the debug switch for objectRegistry is activated, (most probably) the debug output leads to a segmentation fault. I suspect that the debug output code wants to access information already destructed, as the error happens upon destruction of the Time object.

I added a stripped down version of the volField test application to demonstrate this bug in isolation. However, I presume that the bug affects every application that creates a Time object.
Steps To ReproduceAdd this to any controlDict and run any application that creates a Time object.

DebugSwitches
{
    objectRegistry 1;
}
TagsNo tags attached.

Activities

projectionist

2016-03-18 09:00

reporter  

wyldckat

2016-03-19 16:31

updater   ~0006041

Last edited: 2016-03-19 19:52

I was curious about this and here's what I found:

 1- The crash occurs when the destructor "~regIOobject()" calls the inherited "IOobject::path()" method, after the "~Time()" has been fully destroyed.

 2- More specifically, because the "IOobject::path()" method calls "db_.dbDir()", because "instance().isAbsolute() == false". But given that the "db_" has been destroyed, the crash occurs.


The following seems the simplest and most elegant fix I could think of:

    @@ -55,7 +55,7 @@ Foam::objectRegistry::objectRegistry
            IOobject
            (
                string::validate<word>(t.caseName()),
    - "",
    + t.path(),
                t,
                IOobject::NO_READ,
                IOobject::AUTO_WRITE,

Attached is the file "objectRegistry.C" for "src/OpenFOAM/db/objectRegistry/", indexed to the latest OpenFOAM-dev (commit e80917154) (edit: sorry, I forgot to update the copyright date).

This essentially turns the Time-objectRegistry to have a full "instance", resulting in "instance().isAbsolute() == true" and consequently no longer crashing.


Now, the problem is this: I don't know and I can't find any repercussions for this change, given that there could be any somewhat weirdly programmed situations where it's checking for the "instance" to be empty, in order to deduce if it's the main time object or not. A quick search for the calls to "instance()" did not return any such scenarios, but it could be hidden elsewhere.

I could find that the initial object name was only defined as of OpenFOAM 1.5(.0), because before that both path and name were empty. Therefore, I can't be 100% certain as to why was originally defined as an empty string. It does make some sense, since it's the base of the registry tree, but still... the tree should be aware of where it stands :)

wyldckat

2016-03-19 16:31

updater  

objectRegistry.C (8,615 bytes)   
/*---------------------------------------------------------------------------*\
  =========                 |
  \\      /  F ield         | OpenFOAM: The Open Source CFD Toolbox
   \\    /   O peration     |
    \\  /    A nd           | Copyright (C) 2011-2015 OpenFOAM Foundation
     \\/     M anipulation  |
-------------------------------------------------------------------------------
License
    This file is part of OpenFOAM.

    OpenFOAM is free software: you can redistribute it and/or modify it
    under the terms of the GNU General Public License as published by
    the Free Software Foundation, either version 3 of the License, or
    (at your option) any later version.

    OpenFOAM is distributed in the hope that it will be useful, but WITHOUT
    ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
    FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
    for more details.

    You should have received a copy of the GNU General Public License
    along with OpenFOAM.  If not, see <http://www.gnu.org/licenses/>.

\*---------------------------------------------------------------------------*/

#include "objectRegistry.H"
#include "Time.H"

// * * * * * * * * * * * * * * Static Data Members * * * * * * * * * * * * * //

namespace Foam
{
defineTypeNameAndDebug(objectRegistry, 0);
}


// * * * * * * * * * * * * * Private Member Functions  * * * * * * * * * * * //

bool Foam::objectRegistry::parentNotTime() const
{
    return (&parent_ != dynamic_cast<const objectRegistry*>(&time_));
}


// * * * * * * * * * * * * * * * * Constructors *  * * * * * * * * * * * * * //

Foam::objectRegistry::objectRegistry
(
    const Time& t,
    const label nIoObjects
)
:
    regIOobject
    (
        IOobject
        (
            string::validate<word>(t.caseName()),
            t.path(),
            t,
            IOobject::NO_READ,
            IOobject::AUTO_WRITE,
            false
        ),
        true    // to flag that this is the top-level regIOobject
    ),
    HashTable<regIOobject*>(nIoObjects),
    time_(t),
    parent_(t),
    dbDir_(name()),
    event_(1)
{}


Foam::objectRegistry::objectRegistry
(
    const IOobject& io,
    const label nIoObjects
)
:
    regIOobject(io),
    HashTable<regIOobject*>(nIoObjects),
    time_(io.time()),
    parent_(io.db()),
    dbDir_(parent_.dbDir()/local()/name()),
    event_(1)
{
    writeOpt() = IOobject::AUTO_WRITE;
}


// * * * * * * * * * * * * * * * * Destructor  * * * * * * * * * * * * * * * //

Foam::objectRegistry::~objectRegistry()
{
    List<regIOobject*> myObjects(size());
    label nMyObjects = 0;

    for (iterator iter = begin(); iter != end(); ++iter)
    {
        if (iter()->ownedByRegistry())
        {
            myObjects[nMyObjects++] = iter();
        }
    }

    for (label i=0; i < nMyObjects; i++)
    {
        checkOut(*myObjects[i]);
    }
}


// * * * * * * * * * * * * * * * Member Functions  * * * * * * * * * * * * * //

Foam::wordList Foam::objectRegistry::names() const
{
    return HashTable<regIOobject*>::toc();
}


Foam::wordList Foam::objectRegistry::sortedNames() const
{
    return HashTable<regIOobject*>::sortedToc();
}


Foam::wordList Foam::objectRegistry::names(const word& ClassName) const
{
    wordList objectNames(size());

    label count=0;
    for (const_iterator iter = cbegin(); iter != cend(); ++iter)
    {
        if (iter()->type() == ClassName)
        {
            objectNames[count++] = iter.key();
        }
    }

    objectNames.setSize(count);

    return objectNames;
}


Foam::wordList Foam::objectRegistry::sortedNames(const word& ClassName) const
{
    wordList sortedLst = names(ClassName);
    sort(sortedLst);

    return sortedLst;
}


const Foam::objectRegistry& Foam::objectRegistry::subRegistry
(
    const word& name,
    const bool forceCreate
) const
{
    if (forceCreate && !foundObject<objectRegistry>(name))
    {
        objectRegistry* fieldsCachePtr = new objectRegistry
        (
            IOobject
            (
                name,
                time().constant(),
                *this,
                IOobject::NO_READ,
                IOobject::NO_WRITE
            )
        );
        fieldsCachePtr->store();
    }
    return lookupObject<objectRegistry>(name);
}


Foam::label Foam::objectRegistry::getEvent() const
{
    label curEvent = event_++;

    if (event_ == labelMax)
    {
        if (objectRegistry::debug)
        {
            WarningInFunction
                << "Event counter has overflowed. "
                << "Resetting counter on all dependent objects." << nl
                << "This might cause extra evaluations." << endl;
        }

        // Reset event counter
        curEvent = 1;
        event_ = 2;

        for (const_iterator iter = begin(); iter != end(); ++iter)
        {
            const regIOobject& io = *iter();

            if (objectRegistry::debug)
            {
                Pout<< "objectRegistry::getEvent() : "
                    << "resetting count on " << iter.key() << endl;
            }

            if (io.eventNo() != 0)
            {
                const_cast<regIOobject&>(io).eventNo() = curEvent;
            }
        }
    }

    return curEvent;
}


bool Foam::objectRegistry::checkIn(regIOobject& io) const
{
    if (objectRegistry::debug)
    {
        Pout<< "objectRegistry::checkIn(regIOobject&) : "
            << name() << " : checking in " << io.name()
            << endl;
    }

    return const_cast<objectRegistry&>(*this).insert(io.name(), &io);
}


bool Foam::objectRegistry::checkOut(regIOobject& io) const
{
    iterator iter = const_cast<objectRegistry&>(*this).find(io.name());

    if (iter != end())
    {
        if (objectRegistry::debug)
        {
            Pout<< "objectRegistry::checkOut(regIOobject&) : "
                << name() << " : checking out " << iter.key()
                << endl;
        }

        if (iter() != &io)
        {
            if (objectRegistry::debug)
            {
                WarningInFunction
                    << name() << " : attempt to checkOut copy of "
                    << iter.key()
                    << endl;
            }

            return false;
        }
        else
        {
            regIOobject* object = iter();

            bool hasErased = const_cast<objectRegistry&>(*this).erase(iter);

            if (io.ownedByRegistry())
            {
                delete object;
            }

            return hasErased;
        }
    }
    else
    {
        if (objectRegistry::debug)
        {
            Pout<< "objectRegistry::checkOut(regIOobject&) : "
                << name() << " : could not find " << io.name()
                << " in registry " << name()
                << endl;
        }
    }

    return false;
}


void Foam::objectRegistry::rename(const word& newName)
{
    regIOobject::rename(newName);

    // adjust dbDir_ as well
    string::size_type i = dbDir_.rfind('/');

    if (i == string::npos)
    {
        dbDir_ = newName;
    }
    else
    {
        dbDir_.replace(i+1, string::npos, newName);
    }
}


bool Foam::objectRegistry::modified() const
{
    forAllConstIter(HashTable<regIOobject*>, *this, iter)
    {
        if (iter()->modified())
        {
            return true;
        }
    }

    return false;
}


void Foam::objectRegistry::readModifiedObjects()
{
    for (iterator iter = begin(); iter != end(); ++iter)
    {
        if (objectRegistry::debug)
        {
            Pout<< "objectRegistry::readModifiedObjects() : "
                << name() << " : Considering reading object "
                << iter.key() << endl;
        }

        iter()->readIfModified();
    }
}


bool Foam::objectRegistry::readIfModified()
{
    readModifiedObjects();
    return true;
}


bool Foam::objectRegistry::writeObject
(
    IOstream::streamFormat fmt,
    IOstream::versionNumber ver,
    IOstream::compressionType cmp
) const
{
    bool ok = true;

    forAllConstIter(HashTable<regIOobject*>, *this, iter)
    {
        if (objectRegistry::debug)
        {
            Pout<< "objectRegistry::write() : "
                << name() << " : Considering writing object "
                << iter.key()
                << " with writeOpt " << iter()->writeOpt()
                << " to file " << iter()->objectPath()
                << endl;
        }

        if (iter()->writeOpt() != NO_WRITE)
        {
            ok = iter()->writeObject(fmt, ver, cmp) && ok;
        }
    }

    return ok;
}


// ************************************************************************* //
objectRegistry.C (8,615 bytes)   

henry

2016-03-20 10:36

manager   ~0006046

Thanks Bruno
Resolved by commit 9209192b65c13c52002a46df002ef0ffcb1d7ba3

Issue History

Date Modified Username Field Change
2016-03-18 09:00 projectionist New Issue
2016-03-18 09:00 projectionist File Added: Test-TimeCreationDestruciton.tar.gz
2016-03-19 16:31 wyldckat Note Added: 0006041
2016-03-19 16:31 wyldckat File Added: objectRegistry.C
2016-03-19 19:52 wyldckat Note Edited: 0006041
2016-03-20 10:36 henry Note Added: 0006046
2016-03-20 10:36 henry Status new => resolved
2016-03-20 10:36 henry Fixed in Version => dev
2016-03-20 10:36 henry Resolution open => fixed
2016-03-20 10:36 henry Assigned To => henry