Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Infinite loop when std::uncaught_exception is true #495

Closed
tm604 opened this issue Sep 6, 2015 · 5 comments
Closed

Infinite loop when std::uncaught_exception is true #495

tm604 opened this issue Sep 6, 2015 · 5 comments

Comments

@tm604
Copy link

tm604 commented Sep 6, 2015

Been seeing some unusual behaviour with code that uses std::exception_ptr, finally tracked it down to a small-ish test case so thought it might be worth submitting. For anyone who stumbles across this while searching: this one has similar symptoms to #485 but I think the cause is different.

The example code in
https://gist.github.com/tm604/ade0a9354d894a0e71a4 uses std::exception_ptr to store an exception and rethrow it later.

This check:

        if( std::uncaught_exception() ) {
            m_unfinishedSections.push_back( UnfinishedSections( info, prevAssertions, _durationInSeconds ) );
            return;
        }

in https://github.com/philsquared/Catch/blob/1dd0d4c61aa7a5e1bf72fd3c1cca8ab6d7ad5460/include/internal/catch_runner_impl.hpp#L181 is returning true (not entirely sure why it's true, std::current_exception returns false which usually means stack unwinding is still in progress). This is causing an infinite loop because the section is never marked as completed.

Also, probably related: if I remove the REQUIRE/CHECK lines from that test file I get "free(): invalid pointer: 0x0000000001d07c68" instead of an infinite loop - ASAN's not too happy about the code there either (-fsanitize=address), have attached the output as generated when using the v1.2.1 single-header release.

So far I've just been removing that if() block entirely, not sure if there's a better fix - at least that allows the test to complete (and pass, in this case).

cheers,

Tom

tm604 added a commit to tm604/cps-future that referenced this issue Sep 7, 2015
@erhhh
Copy link

erhhh commented Sep 13, 2015

I wound up debugging separately for myself before finding this issue, and figured I should share.

The ASAN issue is likely because this

m_unfinishedSections.push_back( UnfinishedSections( info, prevAssertions, _durationInSeconds ) );

is happening inside this loop

for( std::vector<UnfinishedSections>::const_iterator it = m_unfinishedSections.begin(),
            itEnd = m_unfinishedSections.end();
        it != itEnd;
        ++it )

which is causing it and itEnd to be invalidated. Using std::list instead of std::vector should avoid that.

std::uncaught_exception being true when it shouldn't be is due to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62258

@philsquared
Copy link
Collaborator

Thanks for all the triaging here (sorry it's taken a while to respond).
I've just committed a few changes in this area (on Develop) which should certainly address the vector issue (it should no longer re-enter while iterating) - I'm hoping it will also avoid the uncaught_exception bug (which I'm unable to reproduce, so can't be sure). Please let me know if this change addresses either or both issues for you (I'll be particularly pleased if the uncaught_exception issue goes away without having to resort to the hacks mentioned off that link).

@tm604
Copy link
Author

tm604 commented Sep 28, 2015

Thanks guys - the uncaught_exception is definitely that GCC bug, switching to a patched version of the compiler makes that go away.

The latest version in develop no longer has the infinite loop issue, even without the compiler patch, so that's a good start. However, I'm seeing new test failures with that build - will try to put together a minimal test case for those. It may be related to the uncaught_exception bug again, so I'll try the patched compiler as well to see if that helps.

@philsquared
Copy link
Collaborator

Hi @tm604 - thanks for the report back.
Did you ever get to putting that "minimal test case" for the new failures?
Any clues what the failures are?

@philsquared
Copy link
Collaborator

Any objection to me closing this issue, now? (@tm604 if you're still seeing other issues please raise a new ticket if not related to this one).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants