Skip to content

Conversation

@ihsandemir
Copy link
Collaborator

@ihsandemir ihsandemir commented Dec 20, 2017

Removed connection resets. Added connection alive check to be used in addition to the null check at the ClusterListenerThread thread loop.

The problem case:

  1. Due to connection reset, the connection is destroyed and the underlying receiveBuffer is deleted.
  2. ClusterListenerThread is still at listenMembershipEvents call blocked at the readBlocking call which uses the deleted receiveBuffer of the connection. The receiveBuffer is accessed after the blocking read and this is an illegal memory access.

fixes #369

Attached is the logs that I added which were used at windows to determine where the problem was.
windows_failure_with_logs_added_issue_369.txt

@ihsandemir ihsandemir added this to the 3.9.1 milestone Dec 20, 2017
@ihsandemir ihsandemir self-assigned this Dec 20, 2017
@ihsandemir ihsandemir requested a review from sancar December 20, 2017 10:24
@devOpsHazelcast
Copy link
Contributor

Test PASSed.

1 similar comment
@devOpsHazelcast
Copy link
Contributor

Test PASSed.

if (!isJoined) {
cancel();
join();
CloseHandle(thread);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should not we close the handle if thread is already joined when destructor called ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If the thread is joined already this means that the above code inside if statement executed, hence the handle is closed. Closing the handle twice it may cause as problem as indicated here: https://msdn.microsoft.com/en-us/library/windows/desktop/ms724211(v=vs.85).aspx
"the function will throw an exception if it receives either a handle value that is not valid or a pseudo-handle value. This can happen if you close a handle twice"

@devOpsHazelcast
Copy link
Contributor

Test FAILed.

@devOpsHazelcast
Copy link
Contributor

Test PASSed.

@devOpsHazelcast
Copy link
Contributor

Test FAILed.

@devOpsHazelcast
Copy link
Contributor

Test FAILed.

1 similar comment
@devOpsHazelcast
Copy link
Contributor

Test FAILed.

@devOpsHazelcast
Copy link
Contributor

Test FAILed.

1 similar comment
@devOpsHazelcast
Copy link
Contributor

Test FAILed.

@devOpsHazelcast
Copy link
Contributor

Test PASSed.

1 similar comment
@devOpsHazelcast
Copy link
Contributor

Test PASSed.

… addition to the null check at the ClusterListenerThread thread loop.

The problem case:
1. Due to connection reset, the connection is destroyed and the underlying receiveBuffer is deleted.
2.  ClusterListenerThread is still at listenMembershipEvents call blocked at the readBlocking call which uses the deleted receiveBuffer of the connection. The receiveBuffer is accessed after the blocking read and this is an illegal memory access.
@devOpsHazelcast
Copy link
Contributor

Test PASSed.

1 similar comment
@devOpsHazelcast
Copy link
Contributor

Test PASSed.

@ihsandemir ihsandemir merged commit 74b1bb9 into hazelcast:master Dec 22, 2017
@ihsandemir ihsandemir deleted the memoryAccessFix branch December 22, 2017 09:43
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.

Windows builds fail on client shutdown

3 participants