Skip to content
This repository has been archived by the owner on Jul 21, 2021. It is now read-only.

idempotent Close; queueRequest guards against deadlocks on closed connections #183

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

idempotent Close; queueRequest guards against deadlocks on closed connections #183

wants to merge 3 commits into from

Conversation

jdef
Copy link
Contributor

@jdef jdef commented Jan 13, 2018 •
edited
Loading

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 43.353% when pulling cc6f56c on jdef:jdef/zkclose into 471cd4e on samuel:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+37.6%) to 80.588% when pulling f1083c7 on jdef:jdef/zkclose into 471cd4e on samuel:master.

@jdef jdef changed the title idempotent Close; queueRequest guards against deadlocks on closed connections [--wip--] idempotent Close; queueRequest guards against deadlocks on closed connections Jan 18, 2018
@jdef jdef changed the title [--wip--] idempotent Close; queueRequest guards against deadlocks on closed connections idempotent Close; queueRequest guards against deadlocks on closed connections Jan 18, 2018
@coveralls
Copy link

coveralls commented Jan 18, 2018 •
edited
Loading

Coverage Status

Coverage increased (+0.2%) to 80.266% when pulling dd6d4dd on jdef:jdef/zkclose into c4fab1a on samuel:master.

@jdef
Copy link
Contributor Author

jdef commented Feb 9, 2018

rebased to master

@jdef
Copy link
Contributor Author

jdef commented Feb 9, 2018

@samuel PTAL

@jdef jdef mentioned this pull request Feb 12, 2018
Merged
@atavakoli
Copy link

@samuel We're currently experiencing the #148 panics in our projects and would love to see this merged!

yum-install-brains
yum-install-brains reviewed Feb 25, 2020
View reviewed changes
zk/zk_test.go
@@ -93,6 +93,37 @@ func TestCreate(t *testing.T) {
}
}

func TestOpsAfterCloseDontDeadlock(t *testing.T) {
ts, err := StartTestCluster(1, nil, logWriter{t: t, p: "[ZKERR] "})
Copy link

@yum-install-brains yum-install-brains Feb 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

looks like you've missed t argument
It should be something like
ts, err := StartTestCluster(t, 1, nil, logWriter{t: t, p: "[ZKERR] "})

@grantstephens
Copy link

Any reason this seems to have stalled? Happy to take it over if that is ok?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@yum-install-brains yum-install-brains yum-install-brains left review comments

@samuel samuel samuel 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.

conn.Close is not Idempotent
6 participants
@jdef @coveralls @atavakoli @grantstephens @samuel @yum-install-brains