[#237] Reset one more connection-related state information (missed out in #136) soon after connection gets reset #239
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ssed out in YottaDB#136) soon after connection gets reset Issue YottaDB#136 moved all state related information in gtmrecv_poll_actions.c into a structure variable "curr_conn_state". But there was another global variable "gtmrecv_send_cmp2uncmp" which was set in gtmrecv_process.c and was the same as "curr_conn_state.send_cmp2uncmp" which meant whenever "curr_conn_state.send_cmp2uncmp" got cleared (due to the memset changed introduced in YottaDB#136), "gtmrecv_send_cmp2uncmp" should have also been cleared. Not doing so, meant it was still possible for a stale old connection related state variable to be carried over to a new connection resulting in an assert failure in the source server in rare cases (just like YottaDB#136) in dbg or a connection loss and reconnect sequence in pro (this did happen in one in-house test failure). This is now fixed so both variables are kept in sync. It was originally thought of avoiding the duplication and replacing all usages of "gtmrecv_send_cmp2uncmp" with "curr_conn_state.send_cmp2uncmp" but it was not trivial due to the need to move the "conn_state_t" type definition from gtmsource_poll_actions.c into a header file so gtmrecv_process.c can also inherit that definition. And since this is an edge case, and there was a risk of other subtle regressions, the trivial change was adopted even though it is not as desirable.
estess
approved these changes
May 8, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #136 moved all state related information in gtmrecv_poll_actions.c into a structure
variable "curr_conn_state". But there was another global variable "gtmrecv_send_cmp2uncmp"
which was set in gtmrecv_process.c and was the same as "curr_conn_state.send_cmp2uncmp"
which meant whenever "curr_conn_state.send_cmp2uncmp" got cleared (due to the memset
changed introduced in #136), "gtmrecv_send_cmp2uncmp" should have also been cleared.
Not doing so, meant it was still possible for a stale old connection related state
variable to be carried over to a new connection resulting in an assert failure in the
source server in rare cases (just like #136) in dbg or a connection loss and reconnect
sequence in pro (this did happen in one in-house test failure). This is now fixed
so both variables are kept in sync.
It was originally thought of avoiding the duplication and replacing all usages of
"gtmrecv_send_cmp2uncmp" with "curr_conn_state.send_cmp2uncmp" but it was not trivial
due to the need to move the "conn_state_t" type definition from gtmsource_poll_actions.c
into a header file so gtmrecv_process.c can also inherit that definition. And since this
is an edge case, and there was a risk of other subtle regressions, the trivial change
was adopted even though it is not as desirable.