View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0001538OpenFOAMBugpublic2015-02-17 17:252015-02-21 13:04
Reporterwyldckat Assigned Tohenry  
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
Summary0001538: Undetected ill-formed list can lead PtrList to have a massive memory leak, due to infinite loop
DescriptionI found out about this when using topoSet, when I accidentally and incorrectly closed off the end of the actions list like this:

  actions
  (
  ...
  };

This seems harmless enough, but unfortunately it even lead me to get the machine almost locked up for 30 minutes, while it was filling the swap and I was not able to abort the process (slow swap disk :().

The bug seems to be is in this piece of code at "src/OpenFOAM/containers/Lists/PtrList/PtrListIO.C":

        token lastToken(is);
        while
        (
           !(
                lastToken.isPunctuation()
             && lastToken.pToken() == token::END_LIST
            )
        )
        {
            is.putBack(lastToken);
            sllPtrs.append(inewt(is).ptr());
            is >> lastToken;
        }

When it reaches the last token on the token list/stream, it somewhat strangely gives the last token repeatedly, leading to the infinite loop.
The suggested fix is in the "Additional Information" section.


Further investigation into this lead me to the method "Foam::primitiveEntry::read", where it's parsing the opening and closure of blocks and lists regardless of their types. Nonetheless, trying to account for the two types separately will only avoid the first error triggered, but it would not avoid the second type of error, namely where the user might accidentally swap the list/block terminators, namely 4.b in the next section "Steps To Reproduce".

Steps To Reproduce1. Warning: Do not test this bug, without first running this command:

  ulimit -Sv 2000000

it will limit the amount of RAM allowed for each applications in the current shell.


2. Go to the tutorial "incompressible/simpleFoam/pipeCyclic".

3. Run "blockMesh".

4. Edit the file "system/topoSetDict" and change the end of the list from this:

                option any;
            }
        }
    );

to one of the following two possible-example combinations:

 a)
                option any;
            }
        }
    };


 b)

                option any;
            }
        )
    };


5. Run "topoSet". If you did #1, then it should crash when it reaches the limit of 2GB of RAM. If you did not do #1, then hit Ctrl+C as soon as possible!!!
Additional InformationPossible fix:

--- a/src/OpenFOAM/containers/Lists/PtrList/PtrListIO.C
+++ b/src/OpenFOAM/containers/Lists/PtrList/PtrListIO.C
@@ -116,6 +116,7 @@ void Foam::PtrList<T>::read(Istream& is, const INew& inewt)
         {
             is.putBack(lastToken);
             sllPtrs.append(inewt(is).ptr());
+ is.getBack(lastToken); //flush lastToken
             is >> lastToken;
         }

But this doesn't feel very correct. In fact, this feels very odd as it being a solution... but it worked for me (Ubuntu 12.04, GCC 4.6.3), since it forces to never have anything in the "putBack" buffer, albeit risky.
TagsNo tags attached.

Activities

henry

2015-02-17 23:16

manager   ~0003816

This is an interesting problem. I have looked at various ways to avoid the run-on in the parser and I am not sure your proposal is the best solution. My feeling is that we need to test for EOF while reading the list:

        token lastToken(is);
        while
        (
           !(
                lastToken.isPunctuation()
             && lastToken.pToken() == token::END_LIST
            )
        )
        {
            is.putBack(lastToken);
            if (is.eof())
            {
                FatalIOErrorIn
                (
                    "PtrList<T>::read(Istream&, const INew&)",
                    is
                ) << "Premature EOF after reading " << lastToken.info()
                    << exit(FatalIOError);
            }
            sllPtrs.append(inewt(is).ptr());
            is >> lastToken;
        }

This works well for your test problem but needs to be tested on a wider range of parsing problems to be sure it is the optimal solution.

henry

2015-02-18 08:13

manager   ~0003819

All my tests have shown that the above solution resolves the problem. I have pushed the change into OpenFOAM-2.3.x and OpenFOAM-dev, could you test and confirm?

wyldckat

2015-02-18 11:26

updater   ~0003828

This is a bit odd, because yesterday I had tested using "is.eof()" within the while statement and it didn't work... possibly I used it wrong.

Either way, I'll test this on this coming weekend.
It might be useful to keep this bug report open for others to give some feedback in the meantime as well.

wyldckat

2015-02-21 12:58

updater   ~0003846

I've confirmed that this is working, at least for the tests I reported, along with a few more I tried in "topoSetDict".
One way or another (error+abort or warnings), it no longer has the infinite loop it once had.

henry

2015-02-21 13:04

manager   ~0003847

Resolved by commit 88ac82c68913b458d7310dc8b59da6eccbb34154

Issue History

Date Modified Username Field Change
2015-02-17 17:25 wyldckat New Issue
2015-02-17 23:16 henry Note Added: 0003816
2015-02-18 08:13 henry Note Added: 0003819
2015-02-18 11:26 wyldckat Note Added: 0003828
2015-02-21 12:58 wyldckat Note Added: 0003846
2015-02-21 13:04 henry Note Added: 0003847
2015-02-21 13:04 henry Status new => resolved
2015-02-21 13:04 henry Resolution open => fixed
2015-02-21 13:04 henry Assigned To => henry