rust9x / rust

Empowering everyone to build reliable and efficient software, even for Windows 9x/Me/NT/2000/XP/Vista.
https://github.com/rust9x/rust/wiki
Other
319 stars 9 forks source link

try_lock() always reports "WouldBlock" #21

Closed lbfs closed 3 months ago

lbfs commented 3 months ago

Hi!

I was playing around with this project and seeing if any of my code would work on a platform this old.

I tried this following snippet of code, but it always returns a "WouldBlock" failure. I see that on this version of the compiler, there are a few different Mutex implementations based on what support is available. Is the Mutex implementation that is used on Windows 98 incompatible with this try_lock() method?

    let mutex = Mutex::new(1);
    let guard = mutex.try_lock();

    match guard {
        Ok(_) => {
            println!("Got lock!");
        },
        Err(e) => {
            println!("{:#?}", e);
        }
    }
RandomInsano commented 3 months ago

Just passing through here. I do recall mutexes were a little special on Windows. I did some digging and didn’t quite find what I was looking for. The breaking changes on newer versions of windows were around mutex locks specifically.Random question from me if you’re using the Mutexes in the standard library, have you tried using instead https://docs.rs/parking_lot/latest/parking_lot/ yet?Regards,Edwin

lbfs commented 3 months ago

I'm using the standard library, I am not using a custom mutex implementation.

I tried forcing the different compatibility options for mutex implementations on Windows 98. What I discovered is that the legacy implementation allows my code to work as intended, but when I go into the second implementation ``CriticalSection``` which there is a note about not working on Windows 98 and other early Windows platforms. My application exhibits the same strange behavior it did previously with being unable to obtain a lock.

I think something is possibly wrong with the detection for this method?

https://github.com/rust9x/rust/blob/rust9x/library/std/src/sys/windows/locks/mutex/compat.rs#L21

seritools commented 3 months ago

Awesome, thanks for the analysis.

Win98's kernel32.dll likely has an export for TryEnterCriticalSection that just fails (probably with GetLastError = unsupported), instead of not exporting it at all (just like with all the unicode W versions of APIs that take strings).

So you're right, the check is incorrect. Instead, the check for using CriticalSection for the mutex impl should likely hardcode a check based on https://github.com/rust9x/rust/blob/rust9x/library/std/src/sys/windows/compat/version.rs (= check for NT 4.0+).

For reference, from the old Platform SDK:

TryEnterCriticalSection
  Windows NT: Requires version 4.0 or later.
  Windows: Unsupported.
  Windows CE: Unsupported.
  Header: Declared in winbase.h.
  Import Library: Use kernel32.lib.
seritools commented 3 months ago

@lbfs if you've got time, could you test the fix in https://github.com/rust9x/rust/tree/rust9x-1.76-beta (latest commit)?

Also tracking integrating that fix into the initial impl once rebasing rust9x to a new Rust version in the future #19

lbfs commented 3 months ago

Previous behavior in my application image

With your commit image

So I think the detection is fixed!

Also, in my spare time I found this interesting article seeming to confirm the findings here, also talking about how this could be implemented on previous versions of Windows. https://fanael.github.io/stockfish-on-windows-98.html#implementing-try-enter-critical-section

seritools commented 3 months ago

Haha, I remember that post/that workaround, it certainly should be possible, yeah. Another implementation for this can be found in the KernelEx compatibility layer: https://github.com/metaxor/KernelEx/blob/master/apilibs/kexbases/Kernel32/critsect.c#L161

For rust9x I've been trying to minimize additional, original code where possible, so adding code that relies on such internals is not something I'd want to do ^^

I'll release the new version shortly, likely today.