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

BaseSocket async_write buffer lifecycle [API-1814] [API-1808] [API-1807] #1104

Merged
merged 7 commits into from
Feb 23, 2023

Conversation

akeles85
Copy link
Contributor

@akeles85 akeles85 commented Feb 22, 2023

It seems that there is a problem with the lifecycle of invocations. When async_write method of the boost library is called, user should guarantee the validity of the buffer until the write operation complete. The invocations are added to an unordered_map with add_invocation_to_map method and erased when the response of the message is received or connection is closed. The problem occurs when the invocation is deleted as the response is received, and after that the write handler completion occurs. As the buffer is already deleted, boost async_write completion runs with deleted buffers and error occurs. We think that this case occurs when an async_write occurs while there is already a message in outbox.

A Sample scenario:

  1. Invocation is sent and registered int he invocations map as shared_ptr, which also holds the ClientMessage which in turn holds the data to be transmitted at the wire.
  2. The response is received and invocation is removed from the invocations map and hence the data to be transmitted put at the outbox of the writing side is now invalid pointer.
  3. The handler that is executed after the write completes and at this line, if there is still new message in the queue, the new write operation is being called with the previous invocation, hence that invocation can not protect the life cycle of the new message to be written and this new message may be used after its associated invocation is deleted.

@devOpsHazelcast
Copy link
Contributor

Windows test PASSed.

@devOpsHazelcast
Copy link
Contributor

Linux test PASSed.

Copy link
Contributor

@OzanCansel OzanCansel left a comment

Choose a reason for hiding this comment

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

As a general comment, could you run git-clang-format master to format ?

@devOpsHazelcast
Copy link
Contributor

Windows test PASSed.

@devOpsHazelcast
Copy link
Contributor

Linux test PASSed.

OzanCansel
OzanCansel previously approved these changes Feb 23, 2023
@devOpsHazelcast
Copy link
Contributor

Windows test PASSed.

@devOpsHazelcast
Copy link
Contributor

Linux test PASSed.

@devOpsHazelcast
Copy link
Contributor

Linux test FAILed.

@devOpsHazelcast
Copy link
Contributor

Windows test PASSed.

@devOpsHazelcast
Copy link
Contributor

Linux test PASSed.

@OzanCansel OzanCansel merged commit da69e2d into hazelcast:master Feb 23, 2023
@OzanCansel OzanCansel changed the title BaseSocket async_write buffer lifecycle [API-1814] [API-1808] BaseSocket async_write buffer lifecycle [API-1814] [API-1808] [API-1807] Feb 27, 2023
@devOpsHazelcast
Copy link
Contributor

Linux test PASSed.

@devOpsHazelcast
Copy link
Contributor

Windows test FAILed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants