Mercurial > hg
changeset 43437:93f74a7d3f07
merge with stable
| author | Martin von Zweigbergk <martinvonz@google.com> |
|---|---|
| date | Tue, 05 Nov 2019 13:19:24 -0800 |
| parents | bfc68404cccd (diff) 93aeebc90ff9 (current diff) |
| children | a77d4fe347a4 |
| files | mercurial/commands.py mercurial/pycompat.py |
| diffstat | 13 files changed, 378 insertions(+), 217 deletions(-) [+] |
line wrap: on
line diff
--- a/contrib/perf.py Tue Nov 05 12:10:38 2019 -0500 +++ b/contrib/perf.py Tue Nov 05 13:19:24 2019 -0800 @@ -760,7 +760,10 @@ @command( b'perfstatus', - [(b'u', b'unknown', False, b'ask status to look for unknown files')] + [ + (b'u', b'unknown', False, b'ask status to look for unknown files'), + (b'', b'dirstate', False, b'benchmark the internal dirstate call'), + ] + formatteropts, ) def perfstatus(ui, repo, **opts): @@ -776,7 +779,20 @@ # timer(lambda: sum(map(len, repo.dirstate.status(m, [], False, False, # False)))) timer, fm = gettimer(ui, opts) - timer(lambda: sum(map(len, repo.status(unknown=opts[b'unknown'])))) + if opts[b'dirstate']: + dirstate = repo.dirstate + m = scmutil.matchall(repo) + unknown = opts[b'unknown'] + + def status_dirstate(): + s = dirstate.status( + m, subrepos=[], ignored=False, clean=False, unknown=unknown + ) + sum(map(len, s)) + + timer(status_dirstate) + else: + timer(lambda: sum(map(len, repo.status(unknown=opts[b'unknown'])))) fm.end()
--- a/hgext/censor.py Tue Nov 05 12:10:38 2019 -0500 +++ b/hgext/censor.py Tue Nov 05 13:19:24 2019 -0800 @@ -23,6 +23,9 @@ ``hg update``, must be capable of tolerating censored data to continue to function in a meaningful way. Such commands only tolerate censored file revisions if they are allowed by the "censor.policy=ignore" config option. + +A few informative commands such as ``hg grep`` will unconditionally +ignore censored data and merely report that it was encountered. """ from __future__ import absolute_import
--- a/hgext/largefiles/reposetup.py Tue Nov 05 12:10:38 2019 -0500 +++ b/hgext/largefiles/reposetup.py Tue Nov 05 13:19:24 2019 -0800 @@ -360,20 +360,6 @@ ) return result - def push(self, remote, force=False, revs=None, newbranch=False): - if remote.local(): - missing = set(self.requirements) - remote.local().supported - if missing: - msg = _( - b"required features are not" - b" supported in the destination:" - b" %s" - ) % (b', '.join(sorted(missing))) - raise error.Abort(msg) - return super(lfilesrepo, self).push( - remote, force=force, revs=revs, newbranch=newbranch - ) - # TODO: _subdirlfs should be moved into "lfutil.py", because # it is referred only from "lfutil.updatestandinsbymatch" def _subdirlfs(self, files, lfiles):
--- a/mercurial/commands.py Tue Nov 05 12:10:38 2019 -0500 +++ b/mercurial/commands.py Tue Nov 05 13:19:24 2019 -0800 @@ -3446,6 +3446,9 @@ def grepbody(fn, rev, body): matches[rev].setdefault(fn, []) m = matches[rev][fn] + if body is None: + return + for lnum, cstart, cend, line in matchlines(body): s = linestate(line, lnum, cstart, cend) m.append(s) @@ -3581,6 +3584,19 @@ getrenamed = scmutil.getrenamedfn(repo) + def get_file_content(filename, filelog, filenode, context, revision): + try: + content = filelog.read(filenode) + except error.WdirUnsupported: + content = context[filename].data() + except error.CensoredNodeError: + content = None + ui.warn( + _(b'cannot search in censored file: %(filename)s:%(revnum)s\n') + % {b'filename': filename, b'revnum': pycompat.bytestr(revision)} + ) + return content + def prep(ctx, fns): rev = ctx.rev() pctx = ctx.p1() @@ -3607,17 +3623,15 @@ files.append(fn) if fn not in matches[rev]: - try: - content = flog.read(fnode) - except error.WdirUnsupported: - content = ctx[fn].data() + content = get_file_content(fn, flog, fnode, ctx, rev) grepbody(fn, rev, content) pfn = copy or fn if pfn not in matches[parent]: try: - fnode = pctx.filenode(pfn) - grepbody(pfn, parent, flog.read(fnode)) + pfnode = pctx.filenode(pfn) + pcontent = get_file_content(pfn, flog, pfnode, pctx, parent) + grepbody(pfn, parent, pcontent) except error.LookupError: pass
--- a/mercurial/dirstate.py Tue Nov 05 12:10:38 2019 -0500 +++ b/mercurial/dirstate.py Tue Nov 05 13:19:24 2019 -0800 @@ -687,8 +687,7 @@ delaywrite = self._ui.configint(b'debug', b'dirstate.delaywrite') if delaywrite > 0: # do we have any files to delay for? - items = pycompat.iteritems(self._map) - for f, e in items: + for f, e in pycompat.iteritems(self._map): if e[0] == b'n' and e[3] == now: import time # to avoid useless import @@ -700,12 +699,6 @@ time.sleep(end - clock) now = end # trust our estimate that the end is near now break - # since the iterator is potentially not deleted, - # delete the iterator to release the reference for the Rust - # implementation. - # TODO make the Rust implementation behave like Python - # since this would not work with a non ref-counting GC. - del items self._map.write(st, now) self._lastnormaltime = 0
--- a/mercurial/pycompat.py Tue Nov 05 12:10:38 2019 -0500 +++ b/mercurial/pycompat.py Tue Nov 05 13:19:24 2019 -0800 @@ -94,6 +94,13 @@ import io import struct + if os.name == r'nt' and sys.version_info >= (3, 6): + # MBCS (or ANSI) filesystem encoding must be used as before. + # Otherwise non-ASCII filenames in existing repositories would be + # corrupted. + # This must be set once prior to any fsencode/fsdecode calls. + sys._enablelegacywindowsfsencoding() + fsencode = os.fsencode fsdecode = os.fsdecode oscurdir = os.curdir.encode('ascii') @@ -139,8 +146,8 @@ # # https://hg.python.org/cpython/file/v3.5.1/Programs/python.c#l55 # - # TODO: On Windows, the native argv is wchar_t, so we'll need a different - # workaround to simulate the Python 2 (i.e. ANSI Win32 API) behavior. + # On Windows, the native argv is unicode and is converted to MBCS bytes + # since we do enable the legacy filesystem encoding. if getattr(sys, 'argv', None) is not None: sysargv = list(map(os.fsencode, sys.argv))
--- a/rust/hg-cpython/src/dirstate/copymap.rs Tue Nov 05 12:10:38 2019 -0500 +++ b/rust/hg-cpython/src/dirstate/copymap.rs Tue Nov 05 13:19:24 2019 -0800 @@ -12,7 +12,7 @@ use std::cell::RefCell; use crate::dirstate::dirstate_map::DirstateMap; -use crate::ref_sharing::PyLeakedRef; +use crate::ref_sharing::PyLeaked; use hg::{utils::hg_path::HgPathBuf, CopyMapIter}; py_class!(pub class CopyMap |py| { @@ -104,14 +104,14 @@ py_shared_iterator!( CopyMapKeysIterator, - PyLeakedRef<CopyMapIter<'static>>, + PyLeaked<CopyMapIter<'static>>, CopyMap::translate_key, Option<PyBytes> ); py_shared_iterator!( CopyMapItemsIterator, - PyLeakedRef<CopyMapIter<'static>>, + PyLeaked<CopyMapIter<'static>>, CopyMap::translate_key_value, Option<(PyBytes, PyBytes)> );
--- a/rust/hg-cpython/src/dirstate/dirs_multiset.rs Tue Nov 05 12:10:38 2019 -0500 +++ b/rust/hg-cpython/src/dirstate/dirs_multiset.rs Tue Nov 05 13:19:24 2019 -0800 @@ -17,7 +17,7 @@ }; use crate::dirstate::extract_dirstate; -use crate::ref_sharing::{PyLeakedRef, PySharedRefCell}; +use crate::ref_sharing::{PyLeaked, PySharedRefCell}; use hg::{ utils::hg_path::{HgPath, HgPathBuf}, DirsMultiset, DirsMultisetIter, DirstateMapError, DirstateParseError, @@ -92,7 +92,7 @@ }) } def __iter__(&self) -> PyResult<DirsMultisetKeysIterator> { - let leaked_ref = self.inner_shared(py).leak_immutable()?; + let leaked_ref = self.inner_shared(py).leak_immutable(); DirsMultisetKeysIterator::from_inner( py, unsafe { leaked_ref.map(py, |o| o.iter()) }, @@ -123,7 +123,7 @@ py_shared_iterator!( DirsMultisetKeysIterator, - PyLeakedRef<DirsMultisetIter<'static>>, + PyLeaked<DirsMultisetIter<'static>>, Dirs::translate_key, Option<PyBytes> );
--- a/rust/hg-cpython/src/dirstate/dirstate_map.rs Tue Nov 05 12:10:38 2019 -0500 +++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs Tue Nov 05 13:19:24 2019 -0800 @@ -20,7 +20,7 @@ use crate::{ dirstate::copymap::{CopyMap, CopyMapItemsIterator, CopyMapKeysIterator}, dirstate::{dirs_multiset::Dirs, make_dirstate_tuple}, - ref_sharing::{PyLeakedRef, PySharedRefCell}, + ref_sharing::{PyLeaked, PySharedRefCell}, }; use hg::{ utils::hg_path::{HgPath, HgPathBuf}, @@ -304,7 +304,7 @@ } def keys(&self) -> PyResult<DirstateMapKeysIterator> { - let leaked_ref = self.inner_shared(py).leak_immutable()?; + let leaked_ref = self.inner_shared(py).leak_immutable(); DirstateMapKeysIterator::from_inner( py, unsafe { leaked_ref.map(py, |o| o.iter()) }, @@ -312,7 +312,7 @@ } def items(&self) -> PyResult<DirstateMapItemsIterator> { - let leaked_ref = self.inner_shared(py).leak_immutable()?; + let leaked_ref = self.inner_shared(py).leak_immutable(); DirstateMapItemsIterator::from_inner( py, unsafe { leaked_ref.map(py, |o| o.iter()) }, @@ -320,7 +320,7 @@ } def __iter__(&self) -> PyResult<DirstateMapKeysIterator> { - let leaked_ref = self.inner_shared(py).leak_immutable()?; + let leaked_ref = self.inner_shared(py).leak_immutable(); DirstateMapKeysIterator::from_inner( py, unsafe { leaked_ref.map(py, |o| o.iter()) }, @@ -437,7 +437,7 @@ } def copymapiter(&self) -> PyResult<CopyMapKeysIterator> { - let leaked_ref = self.inner_shared(py).leak_immutable()?; + let leaked_ref = self.inner_shared(py).leak_immutable(); CopyMapKeysIterator::from_inner( py, unsafe { leaked_ref.map(py, |o| o.copy_map.iter()) }, @@ -445,7 +445,7 @@ } def copymapitemsiter(&self) -> PyResult<CopyMapItemsIterator> { - let leaked_ref = self.inner_shared(py).leak_immutable()?; + let leaked_ref = self.inner_shared(py).leak_immutable(); CopyMapItemsIterator::from_inner( py, unsafe { leaked_ref.map(py, |o| o.copy_map.iter()) }, @@ -483,14 +483,14 @@ py_shared_iterator!( DirstateMapKeysIterator, - PyLeakedRef<StateMapIter<'static>>, + PyLeaked<StateMapIter<'static>>, DirstateMap::translate_key, Option<PyBytes> ); py_shared_iterator!( DirstateMapItemsIterator, - PyLeakedRef<StateMapIter<'static>>, + PyLeaked<StateMapIter<'static>>, DirstateMap::translate_key_value, Option<(PyBytes, PyObject)> );
--- a/rust/hg-cpython/src/dirstate/status.rs Tue Nov 05 12:10:38 2019 -0500 +++ b/rust/hg-cpython/src/dirstate/status.rs Tue Nov 05 13:19:24 2019 -0800 @@ -6,7 +6,8 @@ // GNU General Public License version 2 or any later version. //! Bindings for the `hg::status` module provided by the -//! `hg-core` crate. From Python, this will be seen as `rustext.dirstate.status`. +//! `hg-core` crate. From Python, this will be seen as +//! `rustext.dirstate.status`. //! use crate::dirstate::DirstateMap;
--- a/rust/hg-cpython/src/ref_sharing.rs Tue Nov 05 12:10:38 2019 -0500 +++ b/rust/hg-cpython/src/ref_sharing.rs Tue Nov 05 13:19:24 2019 -0800 @@ -23,53 +23,56 @@ //! Macros for use in the `hg-cpython` bridge library. use crate::exceptions::AlreadyBorrowed; -use cpython::{PyClone, PyObject, PyResult, Python}; -use std::cell::{Cell, Ref, RefCell, RefMut}; +use cpython::{exc, PyClone, PyErr, PyObject, PyResult, Python}; +use std::cell::{Ref, RefCell, RefMut}; +use std::ops::{Deref, DerefMut}; +use std::sync::atomic::{AtomicUsize, Ordering}; /// Manages the shared state between Python and Rust +/// +/// `PySharedState` is owned by `PySharedRefCell`, and is shared across its +/// derived references. The consistency of these references are guaranteed +/// as follows: +/// +/// - The immutability of `py_class!` object fields. Any mutation of +/// `PySharedRefCell` is allowed only through its `borrow_mut()`. +/// - The `py: Python<'_>` token, which makes sure that any data access is +/// synchronized by the GIL. +/// - The underlying `RefCell`, which prevents `PySharedRefCell` data from +/// being directly borrowed or leaked while it is mutably borrowed. +/// - The `borrow_count`, which is the number of references borrowed from +/// `PyLeaked`. Just like `RefCell`, mutation is prohibited while `PyLeaked` +/// is borrowed. +/// - The `generation` counter, which increments on `borrow_mut()`. `PyLeaked` +/// reference is valid only if the `current_generation()` equals to the +/// `generation` at the time of `leak_immutable()`. #[derive(Debug, Default)] struct PySharedState { - leak_count: Cell<usize>, - mutably_borrowed: Cell<bool>, + // The counter variable could be Cell<usize> since any operation on + // PySharedState is synchronized by the GIL, but being "atomic" makes + // PySharedState inherently Sync. The ordering requirement doesn't + // matter thanks to the GIL. + borrow_count: AtomicUsize, + generation: AtomicUsize, } -// &PySharedState can be Send because any access to inner cells is -// synchronized by the GIL. -unsafe impl Sync for PySharedState {} - impl PySharedState { fn borrow_mut<'a, T>( &'a self, py: Python<'a>, pyrefmut: RefMut<'a, T>, - ) -> PyResult<PyRefMut<'a, T>> { - if self.mutably_borrowed.get() { - return Err(AlreadyBorrowed::new( - py, - "Cannot borrow mutably while there exists another \ - mutable reference in a Python object", - )); - } - match self.leak_count.get() { + ) -> PyResult<RefMut<'a, T>> { + match self.current_borrow_count(py) { 0 => { - self.mutably_borrowed.replace(true); - Ok(PyRefMut::new(py, pyrefmut, self)) + // Note that this wraps around to the same value if mutably + // borrowed more than usize::MAX times, which wouldn't happen + // in practice. + self.generation.fetch_add(1, Ordering::Relaxed); + Ok(pyrefmut) } - // TODO - // For now, this works differently than Python references - // in the case of iterators. - // Python does not complain when the data an iterator - // points to is modified if the iterator is never used - // afterwards. - // Here, we are stricter than this by refusing to give a - // mutable reference if it is already borrowed. - // While the additional safety might be argued for, it - // breaks valid programming patterns in Python and we need - // to fix this issue down the line. _ => Err(AlreadyBorrowed::new( py, - "Cannot borrow mutably while there are \ - immutable references in Python objects", + "Cannot borrow mutably while immutably borrowed", )), } } @@ -84,41 +87,60 @@ /// extended. Do not call this function directly. unsafe fn leak_immutable<T>( &self, - py: Python, - data: &PySharedRefCell<T>, - ) -> PyResult<(&'static T, &'static PySharedState)> { - if self.mutably_borrowed.get() { - return Err(AlreadyBorrowed::new( - py, - "Cannot borrow immutably while there is a \ - mutable reference in Python objects", - )); - } - // TODO: it's weird that self is data.py_shared_state. Maybe we - // can move stuff to PySharedRefCell? - let ptr = data.as_ptr(); - let state_ptr: *const PySharedState = &data.py_shared_state; - self.leak_count.replace(self.leak_count.get() + 1); - Ok((&*ptr, &*state_ptr)) + _py: Python, + data: Ref<T>, + ) -> (&'static T, &'static PySharedState) { + let ptr: *const T = &*data; + let state_ptr: *const PySharedState = self; + (&*ptr, &*state_ptr) + } + + fn current_borrow_count(&self, _py: Python) -> usize { + self.borrow_count.load(Ordering::Relaxed) + } + + fn increase_borrow_count(&self, _py: Python) { + // Note that this wraps around if there are more than usize::MAX + // borrowed references, which shouldn't happen due to memory limit. + self.borrow_count.fetch_add(1, Ordering::Relaxed); + } + + fn decrease_borrow_count(&self, _py: Python) { + let prev_count = self.borrow_count.fetch_sub(1, Ordering::Relaxed); + assert!(prev_count > 0); } - /// # Safety - /// - /// It's up to you to make sure the reference is about to be deleted - /// when updating the leak count. - fn decrease_leak_count(&self, _py: Python, mutable: bool) { - 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); + fn current_generation(&self, _py: Python) -> usize { + self.generation.load(Ordering::Relaxed) + } +} + +/// Helper to keep the borrow count updated while the shared object is +/// immutably borrowed without using the `RefCell` interface. +struct BorrowPyShared<'a> { + py: Python<'a>, + py_shared_state: &'a PySharedState, +} + +impl<'a> BorrowPyShared<'a> { + fn new( + py: Python<'a>, + py_shared_state: &'a PySharedState, + ) -> BorrowPyShared<'a> { + py_shared_state.increase_borrow_count(py); + BorrowPyShared { + py, + py_shared_state, } } } +impl Drop for BorrowPyShared<'_> { + fn drop(&mut self) { + self.py_shared_state.decrease_borrow_count(self.py); + } +} + /// `RefCell` wrapper to be safely used in conjunction with `PySharedState`. /// /// This object can be stored in a `py_class!` object as a data field. Any @@ -144,15 +166,11 @@ self.inner.borrow() } - fn as_ptr(&self) -> *mut T { - self.inner.as_ptr() - } - // TODO: maybe this should be named as try_borrow_mut(), and use // inner.try_borrow_mut(). The current implementation panics if // self.inner has been borrowed, but returns error if py_shared_state // refuses to borrow. - fn borrow_mut<'a>(&'a self, py: Python<'a>) -> PyResult<PyRefMut<'a, T>> { + fn borrow_mut<'a>(&'a self, py: Python<'a>) -> PyResult<RefMut<'a, T>> { self.py_shared_state.borrow_mut(py, self.inner.borrow_mut()) } } @@ -181,78 +199,31 @@ self.data.borrow(self.py) } - pub fn borrow_mut(&self) -> PyResult<PyRefMut<'a, T>> { + pub fn borrow_mut(&self) -> PyResult<RefMut<'a, T>> { self.data.borrow_mut(self.py) } /// Returns a leaked reference. - pub fn leak_immutable(&self) -> PyResult<PyLeakedRef<&'static T>> { + /// + /// # Panics + /// + /// Panics if this is mutably borrowed. + pub fn leak_immutable(&self) -> PyLeaked<&'static T> { let state = &self.data.py_shared_state; + // make sure self.data isn't mutably borrowed; otherwise the + // generation number can't be trusted. + let data_ref = self.borrow(); unsafe { let (static_ref, static_state_ref) = - state.leak_immutable(self.py, self.data)?; - Ok(PyLeakedRef::new( - self.py, - self.owner, - static_ref, - static_state_ref, - )) + state.leak_immutable(self.py, data_ref); + PyLeaked::new(self.py, self.owner, static_ref, static_state_ref) } } } -/// Holds a mutable reference to data shared between Python and Rust. -pub struct PyRefMut<'a, T> { - py: Python<'a>, - inner: RefMut<'a, T>, - py_shared_state: &'a PySharedState, -} - -impl<'a, T> PyRefMut<'a, T> { - // Must be constructed by PySharedState after checking its leak_count. - // Otherwise, drop() would incorrectly update the state. - fn new( - py: Python<'a>, - inner: RefMut<'a, T>, - py_shared_state: &'a PySharedState, - ) -> Self { - Self { - py, - inner, - py_shared_state, - } - } -} - -impl<'a, T> std::ops::Deref for PyRefMut<'a, T> { - type Target = RefMut<'a, T>; - - fn deref(&self) -> &Self::Target { - &self.inner - } -} -impl<'a, T> std::ops::DerefMut for PyRefMut<'a, T> { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.inner - } -} - -impl<'a, T> Drop for PyRefMut<'a, T> { - fn drop(&mut self) { - self.py_shared_state.decrease_leak_count(self.py, true); - } -} - /// Allows a `py_class!` generated struct to share references to one of its /// data members with Python. /// -/// # Warning -/// -/// TODO allow Python container types: for now, integration with the garbage -/// collector does not extend to Rust structs holding references to Python -/// objects. Should the need surface, `__traverse__` and `__clear__` will -/// need to be written as per the `rust-cpython` docs on GC integration. -/// /// # Parameters /// /// * `$name` is the same identifier used in for `py_class!` macro call. @@ -307,16 +278,22 @@ } /// Manage immutable references to `PyObject` leaked into Python iterators. -pub struct PyLeakedRef<T> { +/// +/// This reference will be invalidated once the original value is mutably +/// borrowed. +pub struct PyLeaked<T> { inner: PyObject, data: Option<T>, py_shared_state: &'static PySharedState, + /// Generation counter of data `T` captured when PyLeaked is created. + generation: usize, } -// DO NOT implement Deref for PyLeakedRef<T>! Dereferencing PyLeakedRef -// without taking Python GIL wouldn't be safe. +// DO NOT implement Deref for PyLeaked<T>! Dereferencing PyLeaked +// without taking Python GIL wouldn't be safe. Also, the underling reference +// is invalid if generation != py_shared_state.generation. -impl<T> PyLeakedRef<T> { +impl<T> PyLeaked<T> { /// # Safety /// /// The `py_shared_state` must be owned by the `inner` Python object. @@ -330,20 +307,39 @@ inner: inner.clone_ref(py), data: Some(data), py_shared_state, + generation: py_shared_state.current_generation(py), } } - /// Returns an immutable reference to the inner value. - pub fn get_ref<'a>(&'a self, _py: Python<'a>) -> &'a T { - self.data.as_ref().unwrap() + /// Immutably borrows the wrapped value. + /// + /// Borrowing fails if the underlying reference has been invalidated. + pub fn try_borrow<'a>( + &'a self, + py: Python<'a>, + ) -> PyResult<PyLeakedRef<'a, T>> { + self.validate_generation(py)?; + Ok(PyLeakedRef { + _borrow: BorrowPyShared::new(py, self.py_shared_state), + data: self.data.as_ref().unwrap(), + }) } - /// Returns a mutable reference to the inner value. + /// Mutably borrows the wrapped value. + /// + /// Borrowing fails if the underlying reference has been invalidated. /// /// Typically `T` is an iterator. If `T` is an immutable reference, /// `get_mut()` is useless since the inner value can't be mutated. - pub fn get_mut<'a>(&'a mut self, _py: Python<'a>) -> &'a mut T { - self.data.as_mut().unwrap() + pub fn try_borrow_mut<'a>( + &'a mut self, + py: Python<'a>, + ) -> PyResult<PyLeakedRefMut<'a, T>> { + self.validate_generation(py)?; + Ok(PyLeakedRefMut { + _borrow: BorrowPyShared::new(py, self.py_shared_state), + data: self.data.as_mut().unwrap(), + }) } /// Converts the inner value by the given function. @@ -351,41 +347,85 @@ /// Typically `T` is a static reference to a container, and `U` is an /// iterator of that container. /// + /// # Panics + /// + /// Panics if the underlying reference has been invalidated. + /// + /// This is typically called immediately after the `PyLeaked` is obtained. + /// In which case, the reference must be valid and no panic would occur. + /// /// # Safety /// /// The lifetime of the object passed in to the function `f` is cheated. /// It's typically a static reference, but is valid only while the - /// corresponding `PyLeakedRef` is alive. Do not copy it out of the + /// corresponding `PyLeaked` is alive. Do not copy it out of the /// function call. pub unsafe fn map<U>( mut self, py: Python, f: impl FnOnce(T) -> U, - ) -> PyLeakedRef<U> { + ) -> PyLeaked<U> { + // Needs to test the generation value to make sure self.data reference + // is still intact. + self.validate_generation(py) + .expect("map() over invalidated leaked reference"); + // f() could make the self.data outlive. That's why map() is unsafe. // In order to make this function safe, maybe we'll need a way to // temporarily restrict the lifetime of self.data and translate the // returned object back to Something<'static>. let new_data = f(self.data.take().unwrap()); - PyLeakedRef { + PyLeaked { inner: self.inner.clone_ref(py), data: Some(new_data), py_shared_state: self.py_shared_state, + generation: self.generation, + } + } + + fn validate_generation(&self, py: Python) -> PyResult<()> { + if self.py_shared_state.current_generation(py) == self.generation { + Ok(()) + } else { + Err(PyErr::new::<exc::RuntimeError, _>( + py, + "Cannot access to leaked reference after mutation", + )) } } } -impl<T> Drop for PyLeakedRef<T> { - fn drop(&mut self) { - // py_shared_state should be alive since we do have - // a Python reference to the owner object. Taking GIL makes - // sure that the state is only accessed by this thread. - let gil = Python::acquire_gil(); - let py = gil.python(); - if self.data.is_none() { - return; // moved to another PyLeakedRef - } - self.py_shared_state.decrease_leak_count(py, false); +/// Immutably borrowed reference to a leaked value. +pub struct PyLeakedRef<'a, T> { + _borrow: BorrowPyShared<'a>, + data: &'a T, +} + +impl<T> Deref for PyLeakedRef<'_, T> { + type Target = T; + + fn deref(&self) -> &T { + self.data + } +} + +/// Mutably borrowed reference to a leaked value. +pub struct PyLeakedRefMut<'a, T> { + _borrow: BorrowPyShared<'a>, + data: &'a mut T, +} + +impl<T> Deref for PyLeakedRefMut<'_, T> { + type Target = T; + + fn deref(&self) -> &T { + self.data + } +} + +impl<T> DerefMut for PyLeakedRefMut<'_, T> { + fn deref_mut(&mut self) -> &mut T { + self.data } } @@ -414,7 +454,7 @@ /// data inner: PySharedRefCell<MyStruct>; /// /// def __iter__(&self) -> PyResult<MyTypeItemsIterator> { -/// let leaked_ref = self.inner_shared(py).leak_immutable()?; +/// let leaked_ref = self.inner_shared(py).leak_immutable(); /// MyTypeItemsIterator::from_inner( /// py, /// unsafe { leaked_ref.map(py, |o| o.iter()) }, @@ -439,7 +479,7 @@ /// /// py_shared_iterator!( /// MyTypeItemsIterator, -/// PyLeakedRef<HashMap<'static, Vec<u8>, Vec<u8>>>, +/// PyLeaked<HashMap<'static, Vec<u8>, Vec<u8>>>, /// MyType::translate_key_value, /// Option<(PyBytes, PyBytes)> /// ); @@ -452,23 +492,14 @@ $success_type: ty ) => { py_class!(pub class $name |py| { - data inner: RefCell<Option<$leaked>>; + data inner: RefCell<$leaked>; def __next__(&self) -> PyResult<$success_type> { - let mut inner_opt = self.inner(py).borrow_mut(); - if let Some(leaked) = inner_opt.as_mut() { - match leaked.get_mut(py).next() { - None => { - // replace Some(inner) by None, drop $leaked - inner_opt.take(); - Ok(None) - } - Some(res) => { - $success_func(py, res) - } - } - } else { - Ok(None) + let mut leaked = self.inner(py).borrow_mut(); + let mut iter = leaked.try_borrow_mut(py)?; + match iter.next() { + None => Ok(None), + Some(res) => $success_func(py, res), } } @@ -484,7 +515,7 @@ ) -> PyResult<Self> { Self::create_instance( py, - RefCell::new(Some(leaked)), + RefCell::new(leaked), ) } } @@ -512,12 +543,94 @@ } #[test] - fn test_borrow_mut_while_leaked() { + fn test_leaked_borrow() { + let (gil, owner) = prepare_env(); + let py = gil.python(); + let leaked = owner.string_shared(py).leak_immutable(); + let leaked_ref = leaked.try_borrow(py).unwrap(); + assert_eq!(*leaked_ref, "new"); + } + + #[test] + fn test_leaked_borrow_mut() { + let (gil, owner) = prepare_env(); + let py = gil.python(); + let leaked = owner.string_shared(py).leak_immutable(); + let mut leaked_iter = unsafe { leaked.map(py, |s| s.chars()) }; + let mut leaked_ref = leaked_iter.try_borrow_mut(py).unwrap(); + assert_eq!(leaked_ref.next(), Some('n')); + assert_eq!(leaked_ref.next(), Some('e')); + assert_eq!(leaked_ref.next(), Some('w')); + assert_eq!(leaked_ref.next(), None); + } + + #[test] + fn test_leaked_borrow_after_mut() { + let (gil, owner) = prepare_env(); + let py = gil.python(); + let leaked = owner.string_shared(py).leak_immutable(); + owner.string_shared(py).borrow_mut().unwrap().clear(); + assert!(leaked.try_borrow(py).is_err()); + } + + #[test] + fn test_leaked_borrow_mut_after_mut() { + let (gil, owner) = prepare_env(); + let py = gil.python(); + let leaked = owner.string_shared(py).leak_immutable(); + let mut leaked_iter = unsafe { leaked.map(py, |s| s.chars()) }; + owner.string_shared(py).borrow_mut().unwrap().clear(); + assert!(leaked_iter.try_borrow_mut(py).is_err()); + } + + #[test] + #[should_panic(expected = "map() over invalidated leaked reference")] + fn test_leaked_map_after_mut() { + let (gil, owner) = prepare_env(); + let py = gil.python(); + let leaked = owner.string_shared(py).leak_immutable(); + owner.string_shared(py).borrow_mut().unwrap().clear(); + let _leaked_iter = unsafe { leaked.map(py, |s| s.chars()) }; + } + + #[test] + fn test_borrow_mut_while_leaked_ref() { let (gil, owner) = prepare_env(); let py = gil.python(); assert!(owner.string_shared(py).borrow_mut().is_ok()); - let _leaked = owner.string_shared(py).leak_immutable().unwrap(); - // TODO: will be allowed - assert!(owner.string_shared(py).borrow_mut().is_err()); + let leaked = owner.string_shared(py).leak_immutable(); + { + let _leaked_ref = leaked.try_borrow(py).unwrap(); + assert!(owner.string_shared(py).borrow_mut().is_err()); + { + let _leaked_ref2 = leaked.try_borrow(py).unwrap(); + assert!(owner.string_shared(py).borrow_mut().is_err()); + } + assert!(owner.string_shared(py).borrow_mut().is_err()); + } + assert!(owner.string_shared(py).borrow_mut().is_ok()); + } + + #[test] + fn test_borrow_mut_while_leaked_ref_mut() { + let (gil, owner) = prepare_env(); + let py = gil.python(); + assert!(owner.string_shared(py).borrow_mut().is_ok()); + let leaked = owner.string_shared(py).leak_immutable(); + let mut leaked_iter = unsafe { leaked.map(py, |s| s.chars()) }; + { + let _leaked_ref = leaked_iter.try_borrow_mut(py).unwrap(); + assert!(owner.string_shared(py).borrow_mut().is_err()); + } + assert!(owner.string_shared(py).borrow_mut().is_ok()); + } + + #[test] + #[should_panic(expected = "mutably borrowed")] + fn test_leak_while_borrow_mut() { + let (gil, owner) = prepare_env(); + let py = gil.python(); + let _mut_ref = owner.string_shared(py).borrow_mut(); + owner.string_shared(py).leak_immutable(); } }
--- a/tests/test-censor.t Tue Nov 05 12:10:38 2019 -0500 +++ b/tests/test-censor.t Tue Nov 05 13:19:24 2019 -0800 @@ -442,6 +442,33 @@ checking files checked 14 changesets with 15 changes to 2 files +Grepping only warns, doesn't error out + + $ cd ../rpull + $ hg grep 'Normal file' + bystander:Normal file v2 + $ hg grep nothing + target:Re-sanitized; nothing to see here + $ hg grep --diff 'Normal file' + cannot search in censored file: target:7 + cannot search in censored file: target:10 + cannot search in censored file: target:12 + bystander:6:-:Normal file v2 + cannot search in censored file: target:1 + cannot search in censored file: target:2 + cannot search in censored file: target:3 + bystander:2:-:Normal file here + bystander:2:+:Normal file v2 + bystander:0:+:Normal file here + $ hg grep --diff nothing + cannot search in censored file: target:7 + cannot search in censored file: target:10 + cannot search in censored file: target:12 + target:13:+:Re-sanitized; nothing to see here + cannot search in censored file: target:1 + cannot search in censored file: target:2 + cannot search in censored file: target:3 + Censored nodes can be imported on top of censored nodes, consecutively $ hg init ../rimport