Merge lp:~mvo/snappy/snappy-panic-less into lp:~snappy-dev/snappy/snappy-moved-to-github

Proposed by Michael Vogt
Status: Needs review
Proposed branch: lp:~mvo/snappy/snappy-panic-less
Merge into: lp:~snappy-dev/snappy/snappy-moved-to-github
Diff against target: 38 lines (+9/-4)
2 files modified
priv/priv.go (+0/-3)
priv/priv_test.go (+9/-1)
To merge this branch: bzr merge lp:~mvo/snappy/snappy-panic-less
Reviewer Review Type Date Requested Status
John Lenton (community) Approve
Review via email: mp+275280@code.launchpad.net

Description of the change

Tiny branch that fixes a panic when doing:
"""
priv := New(lockfile)
priv.Lock()
priv.Unlock()
priv.Lock()
"""

Kudos to John Lenton for discovering this isssue.

To post a comment you must log in.
Revision history for this message
John Lenton (chipaca) wrote :

Hah.

I didn't do this myself because I didn't understand why it was being nil'ed, so left it alone.

That it can be removed entirely is good news :)

review: Approve
Revision history for this message
Michael Vogt (mvo) wrote :

On Thu, Oct 22, 2015 at 09:38:50AM -0000, John Lenton wrote:
> Review: Approve
>
> Hah.
>
> I didn't do this myself because I didn't understand why it was being nil'ed, so left it alone.
>
> That it can be removed entirely is good news :)

I don't know either (I didn't write the code), but looking at the code
I don't think its required (famous last words) so lets fix it and see
what breaks.

Thanks,
 Michael

Unmerged revisions

794. By Michael Vogt

do not panic if a priv.Mutex is taken/released/taken again

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'priv/priv.go'
--- priv/priv.go 2015-06-17 18:34:42 +0000
+++ priv/priv.go 2015-10-22 08:36:17 +0000
@@ -100,9 +100,6 @@
100 return err100 return err
101 }101 }
102102
103 // invalidate
104 m.lock = nil
105
106 return nil103 return nil
107}104}
108105
109106
=== modified file 'priv/priv_test.go'
--- priv/priv_test.go 2015-06-15 07:32:58 +0000
+++ priv/priv_test.go 2015-10-22 08:36:17 +0000
@@ -73,7 +73,6 @@
7373
74 err = privMutex.Unlock()74 err = privMutex.Unlock()
75 c.Assert(err, IsNil)75 c.Assert(err, IsNil)
76 c.Assert(privMutex.lock, IsNil)
77 c.Assert(helpers.FileExists(lockfile), Equals, false)76 c.Assert(helpers.FileExists(lockfile), Equals, false)
78}77}
7978
@@ -141,3 +140,12 @@
141 // and we did not call it too often140 // and we did not call it too often
142 c.Assert(callCount, Equals, 1)141 c.Assert(callCount, Equals, 1)
143}142}
143
144func (ts *PrivTestSuite) TestPrivLockLock(c *C) {
145 lockfile := filepath.Join(ts.tempdir, "lock")
146 priv := New(lockfile)
147 for i := 0; i < 3; i++ {
148 priv.Lock()
149 priv.Unlock()
150 }
151}

Subscribers

People subscribed via source and target branches