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.

Sign up for GitHub

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

Jump to bottom

Only accepting ALIVE changes from unknown trusted writers <1.9.x> [7586] #1005

Merged
merged 2 commits into from
Feb 13, 2020
Merged

Only accepting ALIVE changes from unknown trusted writers <1.9.x> [7586] #1005

merged 2 commits into from
Feb 13, 2020

Conversation

MiguelCompany
Copy link
Member

When a participant with a fixed id is deleted and recreated on the same process, other participants may ignore him because:

  • When being deleted, more than one DATA(p)[UD] may be sent, and on the other participant
    • The first one makes the matched_writer be removed, and history_record is correctly updated
    • When the second one is processed, it is accepted (as the writer id is trusted) and the guid of the writer is added to the history_record with a sequence number of (usually) 2. It is passed to the PDPListener callback, which will do nothing, as it refers to an already removed participant
  • When the participant is created again, the DATA(p) announcing it (with sequence number 1) is received by the other participant, but the history_record has an entry with the same guid and sequence number 2, so the DATA(p) is discarded.

This PR fixes this behavior by only allowing ALIVE changes from unknown trusted writers. With this PR, the second DATA(p)[UD] will be discarded, and then the history_record will not be modified.

@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

IkerLuengo
IkerLuengo approved these changes Feb 13, 2020
View reviewed changes
Copy link
Contributor

@IkerLuengo IkerLuengo left a comment

Choose a reason for hiding this comment

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

LGTM

IkerLuengo added a commit that referenced this pull request Feb 13, 2020
@IkerLuengo
Refs #7552. Only accepting ALIVE changes from unknown trusted writers
4bc6741 
This is a port of #1005 from 1.9.x

* Refs #7552. Only accepting changes from unknown writers when they are ALIVE.
* Refs #7552. Added blackbox test.
@IkerLuengo IkerLuengo mentioned this pull request Feb 13, 2020
Merged
IkerLuengo added a commit that referenced this pull request Feb 13, 2020
@IkerLuengo
Only accepting ALIVE changes from unknown trusted writers
efb404d 
This is a port of #1005 from 1.9.x

 * Refs #7552. Only accepting changes from unknown writers when they are ALIVE.
 * Refs #7552. Added blackbox test.
@IkerLuengo IkerLuengo mentioned this pull request Feb 13, 2020
Merged
MiguelCompany pushed a commit that referenced this pull request Feb 13, 2020
@IkerLuengo
Only accepting ALIVE changes from unknown trusted writers (#1008)
2b7ee84 
This is a port of #1005 from 1.9.x

* Refs #7552. Only accepting changes from unknown writers when they are ALIVE.
* Refs #7552. Added blackbox test.
MiguelBarro
MiguelBarro approved these changes Feb 13, 2020
View reviewed changes
Copy link
Contributor

@MiguelBarro MiguelBarro left a comment

Choose a reason for hiding this comment

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

Maybe we should rename m_acceptMessagesFromUnknownWriters to become m_acceptAliveMessagesFromUnknownWriters :-)

MiguelCompany pushed a commit that referenced this pull request Feb 13, 2020
@IkerLuengo
Only accepting ALIVE changes from unknown trusted writers (#1010)
aa4e855 
This is a port of #1005 from 1.9.x

 * Refs #7552. Only accepting changes from unknown writers when they are ALIVE.
 * Refs #7552. Added blackbox test.
@MiguelCompany
Copy link
Member Author

Reuilding after changes on base branch

@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@MiguelCompany MiguelCompany merged commit 2577261 into 1.9.x Feb 13, 2020
@MiguelCompany MiguelCompany deleted the bugfix/fixed-participant-id/1.9.x branch February 13, 2020 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IkerLuengo IkerLuengo IkerLuengo approved these changes

@MiguelBarro MiguelBarro MiguelBarro approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@MiguelCompany @richiware @MiguelBarro @IkerLuengo