Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Panic broken on Windows XP #34538

Closed
arcnmx opened this issue Jun 28, 2016 · 29 comments
Closed

Panic broken on Windows XP #34538

arcnmx opened this issue Jun 28, 2016 · 29 comments
Labels
A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows C-bug Category: This is a bug. O-windows Operating system: Windows T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@arcnmx
Copy link
Contributor

arcnmx commented Jun 28, 2016

On Windows XP, the program fn main() { panic!() } will fail to run with the following:

thread panicked while processing panic. aborting.

I understand that XP isn't necessarily supported by the standard library, and certain sync implementations are stubbed out (probably the root cause of this issue?), but panicking seems like an operation that should really work if at all possible...

Tested with i686-pc-windows-gnu target and mingw-w64 on:

rustc 1.11.0-nightly (ad7fe6521 2016-06-23)
binary: rustc
commit-hash: ad7fe6521b8a59d84102113ad660edb21de2cba6
commit-date: 2016-06-23
host: x86_64-unknown-linux-gnu
release: 1.11.0-nightly
@alexcrichton
Copy link
Member

Indeed this should definitely work! Would you be able to debug a bit and explore why the double panic is happening? (e.g. the panic messages that were printed or the stack traces)

@alexcrichton
Copy link
Member

I'm also not sure I've ever heard of pc-windows-gnu working on XP, so MSVC may work better. Even then though we don't regularly test so it may still have regressed.

@arcnmx
Copy link
Contributor Author

arcnmx commented Jun 28, 2016

@alexcrichton

Hm, I can try to see if I can install a debugger and get more info, but I did find notify-rs/notify#79 (comment) which seems to include a stack trace of the same issue happening with a double panic.

I'm also not sure I've ever heard of pc-windows-gnu working on XP, so MSVC may work better

It seems to work well enough here, besides the aforementioned issue. If I don't trigger a panic most basic functionality seems fine! I don't have a MSVC Rust dev setup, but if anyone wants to compile the minimal test program and attach an exe I can try running it!

@alexcrichton
Copy link
Member

Aha, that'd do it. When a thread panics it attempts to run its panic hook, but that panic hook is protected by an RwLock which is in turn not implemented on XP. That means that when panicking you'll panic immediately as you attempt to acquire an rwlock.

We could change this to use a mutex somehow perhaps which I believe is implemented on XP at least.

@alexcrichton alexcrichton added the O-windows Operating system: Windows label Jun 29, 2016
@retep998
Copy link
Member

retep998 commented Jun 29, 2016

Alternatively all of Rust's synchronization primitives could be replaced by versions from parking_lot which works on XP (and is faster). cc @Amanieu

@steveklabnik
Copy link
Member

Triage: no changes I'm aware of

@mbilker
Copy link

mbilker commented Nov 9, 2018

I found that this happens to me too. I may go and modify the panic handler to use a Mutex rather than an RWLock so I can find out why this program I am writing is panic-ing on Windows XP Embedded.

@mbilker
Copy link

mbilker commented Nov 9, 2018

Would there be too much of a performance hit if RWLock checks for SRW lock support at runtime like Mutex? Could always implement it and do a crater run to find out myself.

Mutexes work on XP since it falls back to using CriticalSection* functions.

@jonas-schievink
Copy link
Contributor

Triage: #56410 will switch libstd to use parking_lot, but it's still got a long way to go

@mbilker
Copy link

mbilker commented Jul 29, 2019

I've been using the changes from #56410 in a small program running on Windows XP Embedded machines for a few months now with working panic messages.

@arcnmx
Copy link
Contributor Author

arcnmx commented Jul 29, 2019

(I love that most of the interest in XP and the reason this issue was filed seems to stem from one certain company's use of XP Embedded in their arcade cabinets)

@RalfJung
Copy link
Member

Would there be too much of a performance hit if RWLock checks for SRW lock support at runtime like Mutex? Could always implement it and do a crater run to find out myself.

An alternative would be to make RwLock fall back to EnterCriticalSection similar to what Mutex already does. Yes that would mean it's not an actual reader-writer lock (there'd be no concurrent readers, though many readers from the same thread would still work), but that still seems better than panicking...

@jonas-schievink jonas-schievink added A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Mar 11, 2020
@mbilker
Copy link

mbilker commented Mar 13, 2020

An alternative would be to make RwLock fall back to EnterCriticalSection similar to what Mutex already does. Yes that would mean it's not an actual reader-writer lock (there'd be no concurrent readers, though many readers from the same thread would still work), but that still seems better than panicking...

In a previous issue regarding Windows XP fallback implementations, MrAlert did this a little bit lower-level so it is definitely possible [1].

[1] #26654 (comment)

@mbilker
Copy link

mbilker commented Mar 13, 2020

Though, after looking at that implementation again, I would not use it because the keyed events APIs are undocumented.

@mbilker
Copy link

mbilker commented Mar 17, 2020

@RalfJung I decided to follow your advise and threw together a PoC using a wrapper around ReentrantMutex as a fallback when SRW functions are not available using a similar method to what the Windows' Mutex implementation is doing.

https://github.com/mbilker/rust/tree/srwlock-compat-mutex
mbilker@9d5febc

@RalfJung
Copy link
Member

@mbilker nice, that is somewhat like what I had in mind. :)

I imagine the underlying "reentrant mutex" can be shared with the Mutex implementation, to avoid duplicating all that kind logic? Basically, a Windows Mutex could just be a wrapper around RwLock that only exposes write locks. (That is effectively how they are already implemented on Windows Vista+: they use AcquireSRWLockExclusive.)

@mbilker
Copy link

mbilker commented Mar 19, 2020

@RalfJung It is using the ReentrantMutex code from Mutex.

pub struct ReentrantMutex {
inner: UnsafeCell<MaybeUninit<c::CRITICAL_SECTION>>,
}
unsafe impl Send for ReentrantMutex {}
unsafe impl Sync for ReentrantMutex {}
impl ReentrantMutex {
pub fn uninitialized() -> ReentrantMutex {
ReentrantMutex { inner: UnsafeCell::new(MaybeUninit::uninit()) }
}
pub unsafe fn init(&mut self) {
c::InitializeCriticalSection((&mut *self.inner.get()).as_mut_ptr());
}
pub unsafe fn lock(&self) {
c::EnterCriticalSection((&mut *self.inner.get()).as_mut_ptr());
}
#[inline]
pub unsafe fn try_lock(&self) -> bool {
c::TryEnterCriticalSection((&mut *self.inner.get()).as_mut_ptr()) != 0
}
pub unsafe fn unlock(&self) {
c::LeaveCriticalSection((&mut *self.inner.get()).as_mut_ptr());
}
pub unsafe fn destroy(&self) {
c::DeleteCriticalSection((&mut *self.inner.get()).as_mut_ptr());
}
}

@RalfJung
Copy link
Member

What I meant is that all of this code is now essentially duplicated:

impl Mutex {
pub const fn new() -> Mutex {
Mutex {
// This works because SRWLOCK_INIT is 0 (wrapped in a struct), so we are also properly
// initializing an SRWLOCK here.
lock: AtomicUsize::new(0),
held: UnsafeCell::new(false),
}
}
#[inline]
pub unsafe fn init(&mut self) {}
pub unsafe fn lock(&self) {
match kind() {
Kind::SRWLock => c::AcquireSRWLockExclusive(raw(self)),
Kind::CriticalSection => {
let re = self.remutex();
(*re).lock();
if !self.flag_locked() {
(*re).unlock();
panic!("cannot recursively lock a mutex");
}
}
}
}
pub unsafe fn try_lock(&self) -> bool {
match kind() {
Kind::SRWLock => c::TryAcquireSRWLockExclusive(raw(self)) != 0,
Kind::CriticalSection => {
let re = self.remutex();
if !(*re).try_lock() {
false
} else if self.flag_locked() {
true
} else {
(*re).unlock();
false
}
}
}
}
pub unsafe fn unlock(&self) {
*self.held.get() = false;
match kind() {
Kind::SRWLock => c::ReleaseSRWLockExclusive(raw(self)),
Kind::CriticalSection => (*self.remutex()).unlock(),
}
}
pub unsafe fn destroy(&self) {
match kind() {
Kind::SRWLock => {}
Kind::CriticalSection => match self.lock.load(Ordering::SeqCst) {
0 => {}
n => {
Box::from_raw(n as *mut ReentrantMutex).destroy();
}
},
}
}
unsafe fn remutex(&self) -> *mut ReentrantMutex {
match self.lock.load(Ordering::SeqCst) {
0 => {}
n => return n as *mut _,
}
let mut re = box ReentrantMutex::uninitialized();
re.init();
let re = Box::into_raw(re);
match self.lock.compare_and_swap(0, re as usize, Ordering::SeqCst) {
0 => re,
n => {
Box::from_raw(re).destroy();
n as *mut _
}
}
}
unsafe fn flag_locked(&self) -> bool {
if *self.held.get() {
false
} else {
*self.held.get() = true;
true
}
}
}
fn kind() -> Kind {
static KIND: AtomicUsize = AtomicUsize::new(0);
let val = KIND.load(Ordering::SeqCst);
if val == Kind::SRWLock as usize {
return Kind::SRWLock;
} else if val == Kind::CriticalSection as usize {
return Kind::CriticalSection;
}
let ret = match compat::lookup("kernel32", "AcquireSRWLockExclusive") {
None => Kind::CriticalSection,
Some(..) => Kind::SRWLock,
};
KIND.store(ret as usize, Ordering::SeqCst);
ret
}

@mbilker
Copy link

mbilker commented Mar 19, 2020

Ah yeah true. I now have Mutex using RWLock but will need to run the test suite to ensure this works alright. I also discovered the Windows Condvar implementation uses SleepConditionVariableSRW so that will need to be corrected to use SleepConditionVariableCS when RWLock is backed by critical sections.

@mbilker
Copy link

mbilker commented Apr 8, 2020

Sorry for the delay. Are there any testing project out there for testing the Rust synchronization APIs?

@RalfJung
Copy link
Member

RalfJung commented Apr 8, 2020

Well as a start there is the RwLock test suite. Mutex also has one.

@mbilker
Copy link

mbilker commented Apr 8, 2020

So testing my changes would be with python x.py test src/libstd/sync?

EDIT: It was python x.py test src/libstd. Looks like all the tests pass.

@RalfJung
Copy link
Member

RalfJung commented Apr 9, 2020

On Linux I'd do ./x.py test --stage 0 --no-doc src/libstd. The --stage 0 avoids building the entire compiler, twice, just to run some libstd tests.

@mbilker
Copy link

mbilker commented Apr 20, 2020

Considering the nature of this issue, if a panic occurs and the dbghelp.dll is the stock DLL included with Windows XP, then SymInitializeW is not available, only SymInitialize, causing a double panic.

@mbilker
Copy link

mbilker commented Apr 20, 2020

This was after some testing, if I copied a slightly newer dbghelp.dll over into the DLL search path (in my case, the same directory as the executable), then no issues whatsoever. I do need some way to handle recursive locking or follow the old behavior and use a writer count of some sort since that's how EnterCriticalSection works.

@mbilker
Copy link

mbilker commented Apr 21, 2020

There's also some other functions that are not available so I am in the middle of putting in the necessary changes into backtrace-rs so it doesn't attempt to unwrap on missing symbols.

@jeroen
Copy link

jeroen commented May 11, 2020

Hi all,

I would like to build a binary for librsvg that can run on Windows 2008 (Vista). I am going to try to build a custom version of rust just to accomplish this.

Currently on Vista/2008 we get the rust dll error that about missing SetThreadErrorMode in the system api. Would a patch like this solve the issue? https://github.com/jeroen/rust/pull/1

@RalfJung
Copy link
Member

RalfJung commented May 11, 2020

@jeroen This bug here is about Windows XP specifically, because it lacks SRW (slim reader writer) locks. Vista+ have SRW locks so your problem is likely unrelated. Please open a new issue.

@wesleywiser
Copy link
Member

Closing as Windows XP is no longer supported rust-lang/compiler-team#378

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows C-bug Category: This is a bug. O-windows Operating system: Windows T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

10 participants