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

"build directory locked" should say which process is holding the lock #107077

Closed
jyn514 opened this issue Jan 19, 2023 · 14 comments
Closed

"build directory locked" should say which process is holding the lock #107077

jyn514 opened this issue Jan 19, 2023 · 14 comments
Assignees
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@jyn514
Copy link
Member

jyn514 commented Jan 19, 2023

Since #88310, we don't allow multiple processes of bootstrap to run in parallel. That's good to avoid issues like #76661, but means that people sometimes don't know why the build directory is locked (e.g. if they're running rust-analyzer in the background or running x in two terminals at once). We should make it easier to figure this out.

In the short-term, ps -u $USER | grep bootstrap/debug/bootstrap on Unix will show all the relevant processes. In the long-term, and to support windows, we might want to run the equivalent of that ourselves and directly print all relevant PIDs.

@jyn514 jyn514 added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself labels Jan 19, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 23, 2023
…, r=albertlarsan68

Print PID holding bootstrap build lock on Linux

Partially address rust-lang#107077

Parse `/proc/locks` to find the PID of the process which created the build directory lock
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 25, 2023
…, r=albertlarsan68

Print PID holding bootstrap build lock on Linux

Partially address rust-lang#107077

Parse `/proc/locks` to find the PID of the process which created the build directory lock
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 26, 2023
…, r=albertlarsan68

Print PID holding bootstrap build lock on Linux

Partially address rust-lang#107077

Parse `/proc/locks` to find the PID of the process which created the build directory lock
@zephaniahong
Copy link
Contributor

@clubby789 Hi! I saw that you implemented the solution for Linux. May I know why your solution would not work for Mac/Windows?
Also, if I'm trying if implement it for mac/windows, is there anything I should be taking note of?
(I'm a bit new but I'm slowly trying to solve more complex tasks)

Thank you!

@albertlarsan68
Copy link
Member

Windows enthousiast here, quite sarcastic:
The solution for Linux was in the most Linux-y way, eg read a file that only exists in the head of the kernel and extract the information from it.
End of Windows user part, start of Rustacean:
The best way to do this is to do it in a platform-specifix way. If the Linux way also works on MacOS, then the change is trivial, but for Windows the solution will almost certainly be to call Win32 APIs and extract the information from this C-centric API.

@zephaniahong
Copy link
Contributor

Okay! Hope I'm not biting more than I can chew here but I'll give this a shot!
@rustbot claim

@jyn514
Copy link
Member Author

jyn514 commented Feb 11, 2023

There might be an existing crate that does this, I would prefer that to maintaining more platform specific code.

@zephaniahong
Copy link
Contributor

Okay! Let me go source around. But if you chance upon the crate, do let me know(:
Thank you!

@jyn514
Copy link
Member Author

jyn514 commented Feb 11, 2023

@ChrisDenton suggests using Restart Manager on Windows (which I think is https://learn.microsoft.com/en-us/windows/win32/rstmgr/about-restart-manager ?)

@jyn514
Copy link
Member Author

jyn514 commented Feb 11, 2023

@thomcc suggests we can skip all the target specific stuff and just write the PID to the lockfile when we create it 😍

@albertlarsan68
Copy link
Member

There is still the problem of getting the PID, this is just moving the problem elsewhere (maybe also make it simpler?)

@ChrisDenton
Copy link
Member

Moving the problem elsewhere would definitely simplify things. While it is possible to use the Windows Restart Manager for this it's not really what it's intended for (the alternative is to use undocumented APIs in a similar way to handle.exe).

So, yeah, writing the PID to the lockfile makes sense to me.

@jyn514
Copy link
Member Author

jyn514 commented Feb 11, 2023

@albertlarsan68 https://doc.rust-lang.org/stable/std/process/fn.id.html

@jyn514 jyn514 added E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. labels Feb 11, 2023
@zephaniahong
Copy link
Contributor

zephaniahong commented Feb 14, 2023

@jyn514 I've spent some time meddling around and I think I understand the problem but not quite the solution that is being suggested.

  1. When (in which file) do I create the lockfile and add the PID?
  2. Assuming I do have a lockfile with the PIDs, do I then simply display the PIDs when catching the error?
    _build_lock_guard = match build_lock.try_write() {
    Ok(lock) => lock,
    err => {
    drop(err);
    if let Some(pid) = get_lock_owner(&path) {
    println!("warning: build directory locked by process {pid}, waiting for lock");
    } else {
    println!("warning: build directory locked, waiting for lock");
    }
    t!(build_lock.write())
    }
    };

Thank you!

@jyn514
Copy link
Member Author

jyn514 commented Feb 14, 2023

@zephaniahong to path (the variable on line 25), the same file that's being locked. Only add the PID if you've successfully taken out a write lock. Yes, just display the PID :) like it does already on line 26.

@zephaniahong
Copy link
Contributor

ahh! Thanks! I think I get it now! And to confirm, to get the PID, I use this https://doc.rust-lang.org/stable/std/process/fn.id.html ?

@albertlarsan68
Copy link
Member

This proposal has been implemented and merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants