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

Windows Build Broke 0.3.8 #51

Open
gluax opened this issue Nov 22, 2021 · 9 comments
Open

Windows Build Broke 0.3.8 #51

gluax opened this issue Nov 22, 2021 · 9 comments

Comments

@gluax
Copy link

gluax commented Nov 22, 2021

Please see the issue describe at tokio-rs/mio#1535.

@Thomasdezeeuw
Copy link
Contributor

@yoshuawuyts @faern could you yank 0.3.8 for now? It contains a breaking change of some kind that broke Mio (see issue linked in original comment).

@yoshuawuyts
Copy link
Owner

yes, on it! Give me a few mins

@yoshuawuyts
Copy link
Owner

alright, yanked! Crates.io now resolves to 0.3.7 by default: https://crates.io/crates/miow

@Thomasdezeeuw
Copy link
Contributor

Thanks for the quick response @yoshuawuyts!

@yoshuawuyts
Copy link
Owner

yoshuawuyts commented Nov 22, 2021

As to why this released caused build failures, we merged two feature PRs since the last release:

I'll have to do some digging to figure out what caused the issue exactly.

@Thomasdezeeuw
Copy link
Contributor

My guess would be the switch to windows-sys reading the logs in tokio-rs/mio#1535 (comment). I think a type from winapi was exposed in Miow's public interface.

@yoshuawuyts
Copy link
Owner

Ahh, yes I think I might have found the culprit:

Because we switched from winapi to windows-sys, OVERLAPPED comes from a different crate, which leads to issues. Luckily the two definitions are very similar, and going between winapi and windows-sys is likely reasonably straight forward (see next section for an API comparison).

@Thomasdezeeuw what I'm thinking of doing is releasing the current HEAD of miow as 0.4.0. This shouldn't break any existing builds, as it's semver-incompatible. If you like I can then follow that up with a PR to mio to use the latest miow. What do you think?

OVERLAPPED type definitions

Windows Metadata

This is how OVERLAPPED is defined according to microsoft/win32metadata:

public struct OVERLAPPED {
    [StructLayout(LayoutKind.Explicit)]
    public struct _Anonymous_e__Union {
        public struct _Anonymous_e__Struct {
            public uint Offset;
            public uint OffsetHigh;
        }
        [FieldOffset(0)]
        public _Anonymous_e__Struct Anonymous;
        [FieldOffset(0)]
        public unsafe void* Pointer;
    }
    public UIntPtr Internal;
    public UIntPtr InternalHigh;
    public _Anonymous_e__Union Anonymous;
    public HANDLE hEvent;
}

winapi

pub struct OVERLAPPED {
    pub Internal: ULONG_PTR,
    pub InternalHigh: ULONG_PTR,
    pub u: OVERLAPPED_u,
    pub hEvent: HANDLE,
}
pub struct OVERLAPPED_u(_);

windows-sys

pub struct OVERLAPPED {
    pub Internal: usize,
    pub InternalHigh: usize,
    pub Anonymous: OVERLAPPED_0,
    pub hEvent: HANDLE,
}
pub union OVERLAPPED_0 {
    pub Anonymous: OVERLAPPED_0_0,
    pub Pointer: *mut c_void,
}
pub struct OVERLAPPED_0_0 {
    pub Offset: u32,
    pub OffsetHigh: u32,
}

@Thomasdezeeuw
Copy link
Contributor

@Thomasdezeeuw what I'm thinking of doing is releasing the current HEAD of miow as 0.4.0. This shouldn't break any existing builds, as it's semver-incompatible.

I think that makes sense.

If you like I can then follow that up with a PR to mio to use the latest miow. What do you think?

I'll have to review windows-sys crate so that will take me some time. We also use winapi directly, so it might be painful to have both as dependencies. We'll have to see what the best way forward is there.

@Jake-Shadle
Copy link

Jake-Shadle commented Mar 18, 2022

Oops, I didn't see this discussion, but I posted a PR to replace winapi/ntapi in mio just now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants