Skip to content

Fix exec start api with detach and AttachStdin at same time. fixes #2…#20647

Merged
calavera merged 1 commit intomoby:masterfrom
coolljt0725:fix_20638
Feb 25, 2016
Merged

Fix exec start api with detach and AttachStdin at same time. fixes #2…#20647
calavera merged 1 commit intomoby:masterfrom
coolljt0725:fix_20638

Conversation

@coolljt0725
Copy link
Contributor

If create a exec command with AttachStdin on and start with Detach=true
this will cause daemon crash.
This only happen when use API
fixes #20638

Signed-off-by: Lei Jitang [email protected]

/cc @KostyaSha

@calavera
Copy link
Contributor

LGTM

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a new daemon request here to make sure it's not crashed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpuguy83 In this test, there is no need to add a new daemon request here, because if the daemon is crashed, _, body, err := sockRequestRaw("POST", fmt.Sprintf("/exec/%s/start", createResp.ID), strings.NewReader({"Detach": true}), "application/json") will return error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that will probably happen, but just want to verify by sending a subsequent request to make sure it's still alive.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpuguy83 after some more thoughts, it's better to have this check, I'll do

@coolljt0725
Copy link
Contributor Author

@cpuguy83 @calavera updated

@cpuguy83
Copy link
Member

LGTM

calavera added a commit that referenced this pull request Feb 25, 2016
Fix exec start api with detach and AttachStdin at same time. fixes #2
@calavera calavera merged commit cee4ff1 into moby:master Feb 25, 2016
@coolljt0725 coolljt0725 deleted the fix_20638 branch February 25, 2016 08:01
@tiborvass tiborvass mentioned this pull request Mar 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Exec start kills daemon with nil reference

5 participants