Release state back to the pool on errors #6579

Merged
merged 3 commits into from Nov 18, 2016

Conversation

Projects
None yet
3 participants
Member

babbageclunk commented Nov 17, 2016

In the functions that do more checks after the StatePool.Get but return
the state (so can't use a deferred release because it should outlive the
scope), we need to make sure that the state is returned to the pool when
we return an error and a nil state.

Use the named return values + deferred if err != nil release trick (which
really needs a catchier name) when there's more than one error path.

Includes a drive-by fix for the charmsHandler where I was releasing the
state immediately rather than deferring it, and a formatting fix - it looks like
someone's git doesn't have the pre-push hook set up.

babbageclunk added some commits Nov 17, 2016

Release state back to the pool on errors
In the functions that do more checks after the StatePool.Get but return
the state (so can't use a deferred release because it should outlive the
scope), we need to make sure that the state is returned to the pool when
we return an error and a nil state.

Use the named return values + deferred if err != nil release trick (which
really needs a catchier name) when there's more than one error path.

Includes a drive-by fix for the charmsHandler where I was releasing the
state immediately rather than deferring it.
Formatting fix
It looks like someone's git isn't set up with the pre-push hooks.
Member

babbageclunk commented Nov 17, 2016

!!build!!

axw approved these changes Nov 17, 2016

Member

babbageclunk commented Nov 17, 2016

$$merge$$

Contributor

jujubot commented Nov 17, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented Nov 18, 2016

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/9664

Don't store the state pointer in the return variable
Otherwise it gets overwritten before we can release it when there's an
error.
Member

babbageclunk commented Nov 18, 2016

Storing the state pointer in the return variable meant that an error return would set it to nil before running the defer (which tried to release it).

$$merge$$

Contributor

jujubot commented Nov 18, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit 0450416 into juju:develop Nov 18, 2016

@babbageclunk babbageclunk deleted the babbageclunk:release-st-on-errors branch Nov 18, 2016

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