Skip to content

Conversation

@masaori335
Copy link
Contributor

@masaori335 masaori335 commented Feb 3, 2020

Fix the second issue of #6368.

IIUC, when Http2Stream as a VConnection signal VC_EVENT_ERROR event to the upper layer, a vio should be also given as a second argument.

In the #6368 case, 1) read_vio.cont is nullptr and 2) write_vio.cont is nullptr or VC_EVENT_WRITE_COMPLETE or VC_EVENT_EOS is already sent. Thus, this VC_EVENT_ERROR looks unnecessary.

@masaori335 masaori335 added this to the 10.0.0 milestone Feb 3, 2020
@masaori335 masaori335 self-assigned this Feb 3, 2020
@apache apache deleted a comment from bryancall Feb 3, 2020
Copy link
Member

@shinrich shinrich left a comment

Choose a reason for hiding this comment

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

Seems reasonable. It should fall through to the !sent_write_complete case, and the do_io_close() should signal the _sm if it exists. This is attempting to address an error-ful case which may not ever exists.

@masaori335 masaori335 merged commit 240bb69 into apache:master Feb 9, 2020
@zwoop zwoop modified the milestones: 10.0.0, 9.0.0 Feb 11, 2020
@zwoop
Copy link
Contributor

zwoop commented Feb 11, 2020

Cherry-picked to v9.0.x branch.

@zwoop zwoop modified the milestones: 9.0.0, 8.1.0 Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants