add pid to tokio::net::unix::UCred

[kloenk]

Motivation

Fixed: #2632

Solution

Provided pid as Optionlibc::pid_t so solaris and macos does not break.

[Darksonn]

This is a breaking change:

use tokio::net::unix::UCred;

fn main() {
    let a = UCred {
        uid: 10,
        gid: 10,
    };
}

[kloenk]

This is a breaking change:

use tokio::net::unix::UCred;

fn main() {
    let a = UCred {
        uid: 10,
        gid: 10,
    };
}

Should it be behind a feature flag? How should that be named?

[Darksonn]

A feature flag does not solve the issue. If some code compiles without a certain feature flag, it should also compile with that feature flag enabled. This is because when compiling code using Tokio, every feature required by any dependency is enabled for every dependency, and adding one dependency should not break other dependencies.

This change will have to wait for v0.3 of Tokio.

[Darksonn]

I will close the PR for now and come back to this once we prepare to release v0.3.

[Darksonn]

We are working on v0.3 now. Can you submit a commit that merges in the master branch to your branch?

[kloenk]

We are working on v0.3 now. Can you submit a commit that merges in the master branch to your branch?

rebased it onto master

[Darksonn]

Hi. We made a change to this struct such that changes like this are not breaking changes in the future in #2936. Can you rebase again? Sorry about that.

[Darksonn]

It's a bit awkward that we can't get it under MacOS. What would be missing to change that? Perhaps it's better to not provide the function on mac and not use an Option?

[kloenk]

It's a bit awkward that we can't get it under MacOS. What would be missing to change that? Perhaps it's better to not provide the function on mac and not use an Option?

For me it's a mac. So I don't know enough about macos api, if it is possible

[kloenk]

Will disable it for now under MacOS.

I think since 4.2BSD MacOS also supports getsockopt. But I'm not sure, and don't have anything to test it on.

[kloenk]

As far as I can understand the docs and the source of darwin, it's not possible for us to get the PID of the peer process.
The LOCAL_PEERCRED option was added to macos, but it seems like xucred does not contain a PID.

[kloenk]

Found another option, will try to get something working for darwin

[kloenk]

It breaks, while saying bad address. Not yet sure why

[Darksonn]

Let me know if you figure out that its not possible on MacOS.

[kloenk]

Got it to work on my local MacOS hackintosh.

[taiki-e]

not use an Option?

I don't think this is a good idea, as users must specify complex cfgs (cfg(any(target_os * 2 or 4)) for proper use. Also, even if this function becomes available on more platforms, it is not available to users unless they manually update cfgs.

[taiki-e]

Please change the return type to Option or use #[cfg_attr(docsrs, doc(cfg(any(target_os = ...))))] on pid function to improve docs.

[Darksonn]

I was hoping we could make it work everywhere and get rid of both the option and cfg.

[kloenk]

I didn't find this LOCAL_PEEREPID on another bsd. So no luck there

[taiki-e]

I was hoping we could make it work everywhere and get rid of both the option and cfg.

That is most preferable if that is possible, but I don't think we need to block this PR until it becomes work everywhere. Also, if it becomes work everywhere in the future, we can add a document that says "This function always returns Some since tokio v1.x.y".

[Darksonn]

Fair enough. We can return an Option and document on which OSes it works.

[kloenk]

So readding the Option<pid_t> I removed before?

[Darksonn]

Yes.

[kloenk]

Added it, with a small comment saying, where it is implemented