GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Fix descriptor cache race in LOAD_ATTR method specialization · RustPython/RustPython@e2c097e

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

.cspell.dict
- cpython.txt
.cspell.json
crates
- compiler-core/src
  - bytecode.rs
- vm/src
  - frame.rs

Expand file treeCollapse file tree

Open diff view settings

Collapse file

`‎.cspell.dict/cpython.txt‎`

Copy file name to clipboardExpand all lines: .cspell.dict/cpython.txt

+5-1Lines changed: 5 additions & 1 deletion

Original file line number	Diff line number	Diff line change
`@@ -127,8 +127,8 @@ NEWLOCALS`
`127`	`127`	`newsemlockobject`
`128`	`128`	`nfrees`
`129`	`129`	`nkwargs`
`130`		`-nlocalsplus`
`131`	`130`	`nkwelts`
	`131`	`+nlocalsplus`
`132`	`132`	`Nondescriptor`
`133`	`133`	`noninteger`
`134`	`134`	`nops`
`@@ -160,6 +160,7 @@ pylifecycle`
`160`	`160`	`pymain`
`161`	`161`	`pyrepl`
`162`	`162`	`PYTHONTRACEMALLOC`
	`163`	`+PYTHONUTF`
`163`	`164`	`pythonw`
`164`	`165`	`PYTHREAD_NAME`
`165`	`166`	`releasebuffer`
`@@ -171,9 +172,11 @@ saveall`
`171`	`172`	`scls`
`172`	`173`	`setdict`
`173`	`174`	`setfunc`
	`175`	`+setprofileallthreads`
`174`	`176`	`SETREF`
`175`	`177`	`setresult`
`176`	`178`	`setslice`
	`179`	`+settraceallthreads`
`177`	`180`	`SLOTDEFINED`
`178`	`181`	`SMALLBUF`
`179`	`182`	`SOABI`
`@@ -190,6 +193,7 @@ subparams`
`190`	`193`	`subscr`
`191`	`194`	`sval`
`192`	`195`	`swappedbytes`
	`196`	`+sysdict`
`193`	`197`	`templatelib`
`194`	`198`	`testconsole`
`195`	`199`	`ticketer`

Collapse file

`‎.cspell.json‎`

Copy file name to clipboardExpand all lines: .cspell.json

-5Lines changed: 0 additions & 5 deletions

Original file line number	Diff line number	Diff line change
`@@ -152,11 +152,6 @@`
`152`	`152`	`"IFEXEC",`
`153`	`153`	`// "stat"`
`154`	`154`	`"FIRMLINK",`
`155`		`- // CPython internal names`
`156`		`- "PYTHONUTF",`
`157`		`- "sysdict",`
`158`		`- "settraceallthreads",`
`159`		`- "setprofileallthreads"`
`160`	`155`	`],`
`161`	`156`	`// flagWords - list of words to be always considered incorrect`
`162`	`157`	`"flagWords": [`

Collapse file

`‎crates/compiler-core/src/bytecode.rs‎`

Copy file name to clipboardExpand all lines: crates/compiler-core/src/bytecode.rs

+63-1Lines changed: 63 additions & 1 deletion

Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@ use core::{`
`12`	`12`	`cell::UnsafeCell,`
`13`	`13`	`hash, mem,`
`14`	`14`	`ops::Deref,`
`15`		`- sync::atomic::{AtomicU8, AtomicU16, AtomicUsize, Ordering},`
	`15`	`+ sync::atomic::{AtomicU8, AtomicU16, AtomicU32, AtomicUsize, Ordering},`
`16`	`16`	`};`
`17`	`17`	`use itertools::Itertools;`
`18`	`18`	`use malachite_bigint::BigInt;`
`@@ -415,6 +415,9 @@ pub struct CodeUnits {`
`415`	`415`	`/// Single atomic load/store prevents torn reads when multiple threads`
`416`	`416`	`/// specialize the same instruction concurrently.`
`417`	`417`	`pointer_cache: Box<[AtomicUsize]>,`
	`418`	`+ /// SeqLock counter per instruction cache base for descriptor payload writes.`
	`419`	`+ /// odd = write in progress, even = quiescent.`
	`420`	`+ descriptor_sequences: Box<[AtomicU32]>,`
`418`	`421`	`}`
`419`	`422`
`420`	`423`	`// SAFETY: All cache operations use atomic read/write instructions.`
`@@ -441,10 +444,16 @@ impl Clone for CodeUnits {`
`441`	`444`	`.iter()`
`442`	`445`	`.map(\|c\| AtomicUsize::new(c.load(Ordering::Relaxed)))`
`443`	`446`	`.collect();`
	`447`	`+ let descriptor_sequences = self`
	`448`	`+ .descriptor_sequences`
	`449`	`+ .iter()`
	`450`	`+ .map(\|c\| AtomicU32::new(c.load(Ordering::Relaxed)))`
	`451`	`+ .collect();`
`444`	`452`	`Self {`
`445`	`453`	`units: UnsafeCell::new(units),`
`446`	`454`	`adaptive_counters,`
`447`	`455`	`pointer_cache,`
	`456`	`+ descriptor_sequences,`
`448`	`457`	`}`
`449`	`458`	`}`
`450`	`459`	`}`
`@@ -491,10 +500,15 @@ impl From<Vec<CodeUnit>> for CodeUnits {`
`491`	`500`	`.map(\|_\| AtomicUsize::new(0))`
`492`	`501`	`.collect::<Vec<_>>()`
`493`	`502`	`.into_boxed_slice();`
	`503`	`+ let descriptor_sequences = (0..len)`
	`504`	`+ .map(\|_\| AtomicU32::new(0))`
	`505`	`+ .collect::<Vec<_>>()`
	`506`	`+ .into_boxed_slice();`
`494`	`507`	`Self {`
`495`	`508`	`units: UnsafeCell::new(units),`
`496`	`509`	`adaptive_counters,`
`497`	`510`	`pointer_cache,`
	`511`	`+ descriptor_sequences,`
`498`	`512`	`}`
`499`	`513`	`}`
`500`	`514`	`}`
`@@ -641,6 +655,54 @@ impl CodeUnits {`
`641`	`655`	`self.pointer_cache[index].load(Ordering::Relaxed)`
`642`	`656`	`}`
`643`	`657`
	`658`	`+ #[inline]`
	`659`	`+ pub fn begin_descriptor_read(&self, index: usize) -> u32 {`
	`660`	`+ let mut sequence = self.descriptor_sequences[index].load(Ordering::Acquire);`
	`661`	`+ while (sequence & 1) != 0 {`
	`662`	`+ core::hint::spin_loop();`
	`663`	`+ sequence = self.descriptor_sequences[index].load(Ordering::Acquire);`
	`664`	`+ }`
	`665`	`+ sequence`
	`666`	`+ }`
	`667`	`+`
	`668`	`+ #[inline]`
	`669`	`+ pub fn end_descriptor_read(&self, index: usize, previous: u32) -> bool {`
	`670`	`+ core::sync::atomic::fence(Ordering::Acquire);`
	`671`	`+ self.descriptor_sequences[index].load(Ordering::Relaxed) == previous`
	`672`	`+ }`
	`673`	`+`
	`674`	`+ #[inline]`
	`675`	`+ pub fn begin_descriptor_write(&self, index: usize) {`
	`676`	`+ let sequence = &self.descriptor_sequences[index];`
	`677`	`+ let mut seq = sequence.load(Ordering::Acquire);`
	`678`	`+ loop {`
	`679`	`+ while (seq & 1) != 0 {`
	`680`	`+ core::hint::spin_loop();`
	`681`	`+ seq = sequence.load(Ordering::Acquire);`
	`682`	`+ }`
	`683`	`+ match sequence.compare_exchange_weak(`
	`684`	`+ seq,`
	`685`	`+ seq.wrapping_add(1),`
	`686`	`+ Ordering::AcqRel,`
	`687`	`+ Ordering::Acquire,`
	`688`	`+ ) {`
	`689`	`+ Ok(_) => {`
	`690`	`+ core::sync::atomic::fence(Ordering::Release);`
	`691`	`+ break;`
	`692`	`+ }`
	`693`	`+ Err(observed) => {`
	`694`	`+ core::hint::spin_loop();`
	`695`	`+ seq = observed;`
	`696`	`+ }`
	`697`	`+ }`
	`698`	`+ }`
	`699`	`+ }`
	`700`	`+`
	`701`	`+ #[inline]`
	`702`	`+ pub fn end_descriptor_write(&self, index: usize) {`
	`703`	`+ self.descriptor_sequences[index].fetch_add(1, Ordering::Release);`
	`704`	`+ }`
	`705`	`+`
`644`	`706`	/// Read adaptive counter bits for instruction at `index`.
`645`	`707`	`/// Uses Relaxed atomic load.`
`646`	`708`	`pub fn read_adaptive_counter(&self, index: usize) -> u16 {`

Collapse file

`‎crates/vm/src/frame.rs‎`

Copy file name to clipboardExpand all lines: crates/vm/src/frame.rs

+48-12Lines changed: 48 additions & 12 deletions

Original file line number	Diff line number	Diff line change
`@@ -7030,18 +7030,50 @@ impl ExecutingFrame<'_> {`
`7030`	`7030`	`cache_base: usize,`
`7031`	`7031`	`expected_type_version: u32,`
`7032`	`7032`	`) -> Option<PyObjectRef> {`
`7033`		`- let descr_ptr = self.code.instructions.read_cache_ptr(cache_base + 5);`
`7034`		`- if descr_ptr == 0 {`
`7035`		`- return None;`
`7036`		`- }`
`7037`		`- let cloned = unsafe { PyObject::try_to_owned_from_ptr(descr_ptr as *mut PyObject) }?;`
`7038`		`- if self.code.instructions.read_cache_u32(cache_base + 1) == expected_type_version`
`7039`		`- && self.code.instructions.read_cache_ptr(cache_base + 5) == descr_ptr`
`7040`		`- {`
`7041`		`- Some(cloned)`
`7042`		`- } else {`
`7043`		`- drop(cloned);`
`7044`		`- None`
	`7033`	`+ loop {`
	`7034`	`+ let sequence = self.code.instructions.begin_descriptor_read(cache_base);`
	`7035`	`+ let version_before = self.code.instructions.read_cache_u32(cache_base + 1);`
	`7036`	`+ if version_before != expected_type_version \|\| version_before == 0 {`
	`7037`	`+ if self`
	`7038`	`+ .code`
	`7039`	`+ .instructions`
	`7040`	`+ .end_descriptor_read(cache_base, sequence)`
	`7041`	`+ {`
	`7042`	`+ return None;`
	`7043`	`+ }`
	`7044`	`+ continue;`
	`7045`	`+ }`
	`7046`	`+`
	`7047`	`+ let descr_ptr = self.code.instructions.read_cache_ptr(cache_base + 5);`
	`7048`	`+ if descr_ptr == 0 {`
	`7049`	`+ if self`
	`7050`	`+ .code`
	`7051`	`+ .instructions`
	`7052`	`+ .end_descriptor_read(cache_base, sequence)`
	`7053`	`+ {`
	`7054`	`+ return None;`
	`7055`	`+ }`
	`7056`	`+ continue;`
	`7057`	`+ }`
	`7058`	`+`
	`7059`	`+ let cloned = unsafe { PyObject::try_to_owned_from_ptr(descr_ptr as *mut PyObject) };`
	`7060`	`+ let version_after = self.code.instructions.read_cache_u32(cache_base + 1);`
	`7061`	`+ let ptr_after = self.code.instructions.read_cache_ptr(cache_base + 5);`
	`7062`	`+ let stable = self`
	`7063`	`+ .code`
	`7064`	`+ .instructions`
	`7065`	`+ .end_descriptor_read(cache_base, sequence);`
	`7066`	`+`
	`7067`	`+ if let Some(cloned) = cloned`
	`7068`	`+ && stable`
	`7069`	`+ && version_after == expected_type_version`
	`7070`	`+ && ptr_after == descr_ptr`
	`7071`	`+ {`
	`7072`	`+ return Some(cloned);`
	`7073`	`+ }`
	`7074`	`+ if stable {`
	`7075`	`+ return None;`
	`7076`	`+ }`
`7045`	`7077`	`}`
`7046`	`7078`	`}`
`7047`	`7079`
`@@ -7054,6 +7086,7 @@ impl ExecutingFrame<'_> {`
`7054`	`7086`	`) {`
`7055`	`7087`	`// Publish descriptor cache atomically as a tuple:`
`7056`	`7088`	`// invalidate version first, then write payload, then publish version.`
	`7089`	`+ self.code.instructions.begin_descriptor_write(cache_base);`
`7057`	`7090`	`unsafe {`
`7058`	`7091`	`self.code.instructions.write_cache_u32(cache_base + 1, 0);`
`7059`	`7092`	`self.code`
`@@ -7063,6 +7096,7 @@ impl ExecutingFrame<'_> {`
`7063`	`7096`	`.instructions`
`7064`	`7097`	`.write_cache_u32(cache_base + 1, type_version);`
`7065`	`7098`	`}`
	`7099`	`+ self.code.instructions.end_descriptor_write(cache_base);`
`7066`	`7100`	`}`
`7067`	`7101`
`7068`	`7102`	`#[inline]`
`@@ -7074,6 +7108,7 @@ impl ExecutingFrame<'_> {`
`7074`	`7108`	`descr_ptr: usize,`
`7075`	`7109`	`) {`
`7076`	`7110`	`// Same publish protocol as write_cached_descriptor(), plus metaclass guard.`
	`7111`	`+ self.code.instructions.begin_descriptor_write(cache_base);`
`7077`	`7112`	`unsafe {`
`7078`	`7113`	`self.code.instructions.write_cache_u32(cache_base + 1, 0);`
`7079`	`7114`	`self.code`
`@@ -7086,6 +7121,7 @@ impl ExecutingFrame<'_> {`
`7086`	`7121`	`.instructions`
`7087`	`7122`	`.write_cache_u32(cache_base + 1, type_version);`
`7088`	`7123`	`}`
	`7124`	`+ self.code.instructions.end_descriptor_write(cache_base);`
`7089`	`7125`	`}`
`7090`	`7126`
`7091`	`7127`	`fn load_attr(&mut self, vm: &VirtualMachine, oparg: LoadAttr) -> FrameResult {`

0 commit comments

Comments

0 (0)

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.