Submitter | Yuya Nishihara |
---|---|
Date | Sept. 15, 2019, 4:29 p.m. |
Message ID | <071b35e3f7cde53cbf41.1568564991@mimosa> |
Download | mbox | patch |
Permalink | /patch/41678/ |
State | Accepted |
Headers | show |
Comments
Good idea, thanks! On 9/15/19 6:29 PM, Yuya Nishihara wrote: > # HG changeset patch > # User Yuya Nishihara <yuya@tcha.org> > # Date 1568555012 -32400 > # Sun Sep 15 22:43:32 2019 +0900 > # Node ID 071b35e3f7cde53cbf411a536091153741dc4a37 > # Parent 34ed651ba7e4a4b7f0ee79c5a5442b5eed27bbad > rust-cpython: add sanity check to PySharedState::decrease_leak_count() > > If decrease_leak_count() were called unnecessarily, there must be a serious > bug. It's better to not silently ignore such cases. > > diff --git a/rust/hg-cpython/src/ref_sharing.rs b/rust/hg-cpython/src/ref_sharing.rs > --- a/rust/hg-cpython/src/ref_sharing.rs > +++ b/rust/hg-cpython/src/ref_sharing.rs > @@ -85,10 +85,14 @@ impl PySharedState { > /// It's unsafe to update the reference count without knowing the > /// reference is deleted. Do not call this function directly. > pub unsafe fn decrease_leak_count(&self, _py: Python, mutable: bool) { > - self.leak_count > - .replace(self.leak_count.get().saturating_sub(1)); > if mutable { > + assert_eq!(self.leak_count.get(), 0); > + assert!(self.mutably_borrowed.get()); > self.mutably_borrowed.replace(false); > + } else { > + let count = self.leak_count.get(); > + assert!(count > 0); > + self.leak_count.replace(count - 1); > } > } > } > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
On Sun, Sep 15, 2019 at 7:33 PM Yuya Nishihara <yuya@tcha.org> wrote: > > # HG changeset patch > # User Yuya Nishihara <yuya@tcha.org> > # Date 1568555012 -32400 > # Sun Sep 15 22:43:32 2019 +0900 > # Node ID 071b35e3f7cde53cbf411a536091153741dc4a37 > # Parent 34ed651ba7e4a4b7f0ee79c5a5442b5eed27bbad > rust-cpython: add sanity check to PySharedState::decrease_leak_count() > > If decrease_leak_count() were called unnecessarily, there must be a serious > bug. It's better to not silently ignore such cases. Queued this, many thanks. Thanks to Raphaël for having a look.
Patch
diff --git a/rust/hg-cpython/src/ref_sharing.rs b/rust/hg-cpython/src/ref_sharing.rs --- a/rust/hg-cpython/src/ref_sharing.rs +++ b/rust/hg-cpython/src/ref_sharing.rs @@ -85,10 +85,14 @@ impl PySharedState { /// It's unsafe to update the reference count without knowing the /// reference is deleted. Do not call this function directly. pub unsafe fn decrease_leak_count(&self, _py: Python, mutable: bool) { - self.leak_count - .replace(self.leak_count.get().saturating_sub(1)); if mutable { + assert_eq!(self.leak_count.get(), 0); + assert!(self.mutably_borrowed.get()); self.mutably_borrowed.replace(false); + } else { + let count = self.leak_count.get(); + assert!(count > 0); + self.leak_count.replace(count - 1); } } }