Skip to content
This repository has been archived by the owner on Mar 4, 2021. It is now read-only.

async: adjust tests to expose bug, then fix it #133

Merged
merged 3 commits into from
Jun 19, 2015
Merged

Conversation

bassosimone
Copy link
Member

This pull request adjusts async tests to expose a bug (leading to a crash) spotted while working on the iOS application and then fixes the bug (basically, the issue was that I overwrote the thread object before either detaching or joining it).

No functional change, this diff is needed to prepare for the
following diff that will trigger a crash in async.cpp.
This diff is best reviewed using `git diff -w` feature.

On MacOS, you see something like the following:

    $ ./test/common/async
    ...
    test #1: http: BODY
    test #1: http: received body chunk...
    test #1: http: END
    test #1: http: we have reached end of response
    test #1: net_test: tearing down
    test #1: net_test: written entry
    test #1: net_test: reached end of input
    async: test stopped
    test complete: 140525600582144
    async: bottom of loop thread
    async: loop thread locked
    async: empty
    async: thread stopped
    all tests completed
    do another iteration of tests
    test created: 140525592131072
    async: test inserted
    async: background thread started
    libc++abi.dylib: async: loop thread entered
    terminating
    Abort trap: 6

A similar error is seen on GNU/Linux.
With this fix committed, the code does not crash anymore when
we create another background thread.

Since I observed this both on MacOS and Linux with different
compilers and C++ libraries, it was probably a programmer error
that I not explicitly joined/detached the thread.

Debugging this issue I also noticed memory leaks on Linux
that I am going to investigate.
@bassosimone bassosimone changed the title async: adjust tests to trigger crash, then fix it async: adjust tests to expose bug, then fix it Jun 18, 2015
@hellais
Copy link
Contributor

hellais commented Jun 19, 2015

Smells like hotfix spirit.

I think we can merge this safely.

hellais added a commit that referenced this pull request Jun 19, 2015
async: adjust tests to expose bug, then fix it
@hellais hellais merged commit b2e814f into master Jun 19, 2015
@hellais hellais deleted the fix/async-crash branch June 19, 2015 10:04
@bassosimone
Copy link
Member Author

"I found it hard; it's hard to find" 👍

@hellais
Copy link
Contributor

hellais commented Jun 19, 2015

Oh well, whatever, nevermind

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

Successfully merging this pull request may close these issues.

None yet

2 participants