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

kvs: add tests, comments, and use after free fix #1773

Merged
merged 6 commits into from Oct 26, 2018

Conversation

Projects
None yet
2 participants
@chu11
Copy link
Contributor

chu11 commented Oct 26, 2018

Per #1751 - add some extra tests to cover this specific behavior and add some comments to clarify how some things work.

Per #1760 - add reproducer from @garlick, add fix for use-after-free issue in kvstxn, add unit test for kvstxn.

chu11 and others added some commits Oct 24, 2018

modules/kvs/test: Add missing ref count tests
Add tests in kvstxn to test number of missing references returned
when processing.
modules/kvs: Add additional logging
Add additional logging to content_load_request_send
modules/kvs: Fix use after free in kvstxn
When returning missing refs, strdup the reference before
storing it in the missing refs list.  We cannot guarantee that the memory
storing the reference will exist in the future.  For example, a user
may choose to unlink the reference in a later operation in the same
transaction.

Fixes #1760
modules/kvs/test: Add use after free test
Add unit test to test for specific case where a reference needs
to be loaded, but the user later removes it within the same
transaction.
@chu11

This comment has been minimized.

Copy link
Contributor Author

chu11 commented Oct 26, 2018

hmmm, one builder failed, all tests passed, found one "write error: stdout" error. restarted.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Oct 26, 2018

Thanks @chu11, I'll go ahead and merge this.

@garlick garlick merged commit 8bc27cf into flux-framework:master Oct 26, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.