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

Cyclic dependencies result in deadlock #834

Closed
huonw opened this issue Nov 11, 2014 · 3 comments
Closed

Cyclic dependencies result in deadlock #834

huonw opened this issue Nov 11, 2014 · 3 comments
Labels
E-easy Experience: Easy

Comments

@huonw
Copy link
Member

huonw commented Nov 11, 2014

# ./d/Cargo.toml
[package]

name = "d"
version = "0.0.1"
authors = ["Huon Wilson <dbau.pp+github@gmail.com>"]

[dependencies.e]
path = "../e"
# ./e/Cargo.toml
[package]

name = "e"
version = "0.0.1"
authors = ["Huon Wilson <dbau.pp+github@gmail.com>"]

[dependencies.d]
path = "../d"

with both d/src/lib.rs and e/src/lib.rs empty. RUST_LOG=cargo cargo build in d gives:

5:cargo::sources::path: PathSource::for_path; path=/home/huon/projects/test-rust/tmp/e
5:cargo::sources::path: new; id=file:///home/huon/projects/test-rust/tmp/e
5:cargo::ops::cargo_read_manifest: looking for root package: /home/huon/projects/test-rust/tmp/e, source_id=file:///home/huon/projects/test-rust/tmp/e
5:cargo::ops::cargo_read_manifest: read_package; path=/home/huon/projects/test-rust/tmp/e/Cargo.toml; source-id=file:///home/huon/projects/test-rust/tmp/e
5:cargo::ops::cargo_read_manifest: read_package; path=/home/huon/projects/test-rust/tmp/d/Cargo.toml; source-id=file:///home/huon/projects/test-rust/tmp/e
5:cargo::ops::cargo_read_manifest: looking for child package: /home/huon/projects/test-rust/tmp/e
5:cargo::ops::cargo_read_manifest: not processing /home/huon/projects/test-rust/tmp/e
5:cargo::ops::cargo_read_manifest: all packages: [e v0.0.1 (file:///home/huon/projects/test-rust/tmp/e), d v0.0.1 (file:///home/huon/projects/test-rust/tmp/e)]
5:cargo::sources::path: get_root_package; source=the paths source
5:cargo::core::source: loading SourceId; file:///home/huon/projects/test-rust/tmp/e
5:cargo::sources::path: new; id=file:///home/huon/projects/test-rust/tmp/e
5:cargo::ops::cargo_read_manifest: looking for root package: /home/huon/projects/test-rust/tmp/e, source_id=file:///home/huon/projects/test-rust/tmp/e
5:cargo::ops::cargo_read_manifest: read_package; path=/home/huon/projects/test-rust/tmp/e/Cargo.toml; source-id=file:///home/huon/projects/test-rust/tmp/e
5:cargo::ops::cargo_read_manifest: read_package; path=/home/huon/projects/test-rust/tmp/d/Cargo.toml; source-id=file:///home/huon/projects/test-rust/tmp/e
5:cargo::ops::cargo_read_manifest: looking for child package: /home/huon/projects/test-rust/tmp/e
5:cargo::ops::cargo_read_manifest: not processing /home/huon/projects/test-rust/tmp/e
5:cargo::ops::cargo_read_manifest: all packages: [e v0.0.1 (file:///home/huon/projects/test-rust/tmp/e), d v0.0.1 (file:///home/huon/projects/test-rust/tmp/e)]
5:cargo::core::resolver: resolve; summary=Summary { package_id: e v0.0.1 (file:///home/huon/projects/test-rust/tmp/e), dependencies: [Dependency { name: d, source_id: file:///home/huon/projects/test-rust/tmp/e, req: = 0.0.1, specified_req: None, kind: Normal, only_match_name: false, optional: false, default_features: true, features: [], only_for_platform: None }], features: {} }
5:cargo::core::resolver: e[0]>d 1 candidates
5:cargo::core::resolver: e[0]>d 0 prev activations
5:cargo::core::resolver: e[0]>d trying 0.0.1
5:cargo::core::resolver: d[0]>e 1 candidates
5:cargo::core::resolver: d[0]>e 1 prev activations
5:cargo::core::resolver: d[0]>e trying 0.0.1
5:cargo::core::source: loading SourceId; file:///home/huon/projects/test-rust/tmp/e
5:cargo::sources::path: new; id=file:///home/huon/projects/test-rust/tmp/e
5:cargo::ops::cargo_read_manifest: looking for root package: /home/huon/projects/test-rust/tmp/e, source_id=file:///home/huon/projects/test-rust/tmp/e
5:cargo::ops::cargo_read_manifest: read_package; path=/home/huon/projects/test-rust/tmp/e/Cargo.toml; source-id=file:///home/huon/projects/test-rust/tmp/e
5:cargo::ops::cargo_read_manifest: read_package; path=/home/huon/projects/test-rust/tmp/d/Cargo.toml; source-id=file:///home/huon/projects/test-rust/tmp/e
5:cargo::ops::cargo_read_manifest: looking for child package: /home/huon/projects/test-rust/tmp/e
5:cargo::ops::cargo_read_manifest: not processing /home/huon/projects/test-rust/tmp/e
5:cargo::ops::cargo_read_manifest: all packages: [e v0.0.1 (file:///home/huon/projects/test-rust/tmp/e), d v0.0.1 (file:///home/huon/projects/test-rust/tmp/e)]
5:cargo::core::resolver: resolve; summary=Summary { package_id: e v0.0.1 (file:///home/huon/projects/test-rust/tmp/e), dependencies: [Dependency { name: d, source_id: file:///home/huon/projects/test-rust/tmp/e, req: = 0.0.1, specified_req: None, kind: Normal, only_match_name: false, optional: false, default_features: true, features: [], only_for_platform: None }], features: {} }
5:cargo::core::resolver: e[0]>d 1 candidates
5:cargo::core::resolver: e[0]>d 0 prev activations
5:cargo::core::resolver: e[0]>d trying 0.0.1
5:cargo::core::resolver: d[0]>e 1 candidates
5:cargo::core::resolver: d[0]>e 1 prev activations
5:cargo::core::resolver: d[0]>e trying 0.0.1
5:cargo::core::registry: getting packags; sources=1; ids=[d v0.0.1 (file:///home/huon/projects/test-rust/tmp/e), e v0.0.1 (file:///home/huon/projects/test-rust/tmp/e)]
5:cargo::sources::path: getting packages; ids=[d v0.0.1 (file:///home/huon/projects/test-rust/tmp/e), e v0.0.1 (file:///home/huon/projects/test-rust/tmp/e)]
INFO:cargo::ops::cargo_rustc::fingerprint: fingerprint at: /home/huon/projects/test-rust/tmp/e/target/.fingerprint/d-b56b95a761370601/lib-d
INFO:cargo::ops::cargo_rustc::fingerprint: fingerprint at: /home/huon/projects/test-rust/tmp/e/target/.fingerprint/e-ef8f59a201a8c5af/lib-e
INFO:cargo::ops::cargo_rustc::job_queue: start: e v0.0.1 (file:///home/huon/projects/test-rust/tmp/e) StageStart
INFO:cargo::ops::cargo_rustc::job_queue: start: d v0.0.1 (file:///home/huon/projects/test-rust/tmp/e) StageStart
INFO:cargo::ops::cargo_rustc::job_queue:   end: e v0.0.1 (file:///home/huon/projects/test-rust/tmp/e) StageStart
INFO:cargo::ops::cargo_rustc::job_queue: start: e v0.0.1 (file:///home/huon/projects/test-rust/tmp/e) StageBuildCustomBuild
INFO:cargo::ops::cargo_rustc::job_queue:   end: d v0.0.1 (file:///home/huon/projects/test-rust/tmp/e) StageStart
INFO:cargo::ops::cargo_rustc::job_queue: start: d v0.0.1 (file:///home/huon/projects/test-rust/tmp/e) StageBuildCustomBuild
INFO:cargo::ops::cargo_rustc::job_queue:   end: e v0.0.1 (file:///home/huon/projects/test-rust/tmp/e) StageBuildCustomBuild
INFO:cargo::ops::cargo_rustc::job_queue:   end: d v0.0.1 (file:///home/huon/projects/test-rust/tmp/e) StageBuildCustomBuild

and then it hangs using no CPU. Backtraces:

(gdb) thread apply all bt

Thread 9 (Thread 0x7ffff47ff700 (LWP 24453)):
#0  pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
#1  0x000055555563fc50 in task::Ops.rt..Runtime::deschedule::h84e263a04c9681b0Nde ()
#2  0x0000555555b48461 in task::Task::deschedule::haa3cdecd71575989pVc ()
#3  0x0000555555b4113b in comm::Receiver$LT$T$GT$::recv_opt::h15464787829753008962 ()
#4  0x0000555555b40e75 in raw::Sem$LT$Q$GT$::acquire::h8788800274794914083 ()
#5  0x0000555555b43237 in raw::Mutex::lock::h322feff1785dd894rub ()
#6  0x000055555583507d in util::pool::TaskPool::new::closure.29443 ()
#7  0x0000555555640762 in task::NativeSpawner.Spawner::spawn::closure.8456 ()
#8  0x0000555555b4a58c in rust_try_inner ()
#9  0x0000555555b4a576 in rust_try ()
#10 0x0000555555b47bb3 in unwind::try::h32143614f9ac9c88PGd ()
[...]

Thread 8 (Thread 0x7ffff5ffd700 (LWP 24452)):
#0  pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
#1  0x000055555563fc50 in task::Ops.rt..Runtime::deschedule::h84e263a04c9681b0Nde ()
#2  0x0000555555b48461 in task::Task::deschedule::haa3cdecd71575989pVc ()
#3  0x0000555555b43183 in comm::sync::wait::h96c055f3ae5cefd3HOd ()
#4  0x0000555555835b7f in util::pool::TaskPool::new::closure.29443 ()
#5  0x0000555555640762 in task::NativeSpawner.Spawner::spawn::closure.8456 ()
#6  0x0000555555b4a58c in rust_try_inner ()
#7  0x0000555555b4a576 in rust_try ()
#8  0x0000555555b47bb3 in unwind::try::h32143614f9ac9c88PGd ()
#9  0x0000555555b47a8c in task::Task::run::h8c90cb04437a131dFMc ()
#10 0x0000555555640567 in task::NativeSpawner.Spawner::spawn::closure.8394 ()
[...]

Thread 7 (Thread 0x7ffff63ff700 (LWP 24451)):
#0  pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
#1  0x000055555563fc50 in task::Ops.rt..Runtime::deschedule::h84e263a04c9681b0Nde ()
#2  0x0000555555b48461 in task::Task::deschedule::haa3cdecd71575989pVc ()
#3  0x0000555555b4113b in comm::Receiver$LT$T$GT$::recv_opt::h15464787829753008962 ()
#4  0x0000555555b40e75 in raw::Sem$LT$Q$GT$::acquire::h8788800274794914083 ()
#5  0x0000555555b43237 in raw::Mutex::lock::h322feff1785dd894rub ()
#6  0x000055555583507d in util::pool::TaskPool::new::closure.29443 ()
#7  0x0000555555640762 in task::NativeSpawner.Spawner::spawn::closure.8456 ()
#8  0x0000555555b4a58c in rust_try_inner ()
#9  0x0000555555b4a576 in rust_try ()
#10 0x0000555555b47bb3 in unwind::try::h32143614f9ac9c88PGd ()
[...]

Thread 6 (Thread 0x7ffff61fe700 (LWP 24450)):
#0  pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
#1  0x000055555563fc50 in task::Ops.rt..Runtime::deschedule::h84e263a04c9681b0Nde ()
#2  0x0000555555b48461 in task::Task::deschedule::haa3cdecd71575989pVc ()
#3  0x0000555555b4113b in comm::Receiver$LT$T$GT$::recv_opt::h15464787829753008962 ()
#4  0x0000555555b40e75 in raw::Sem$LT$Q$GT$::acquire::h8788800274794914083 ()
#5  0x0000555555b43237 in raw::Mutex::lock::h322feff1785dd894rub ()
#6  0x000055555583507d in util::pool::TaskPool::new::closure.29443 ()
#7  0x0000555555640762 in task::NativeSpawner.Spawner::spawn::closure.8456 ()
#8  0x0000555555b4a58c in rust_try_inner ()
#9  0x0000555555b4a576 in rust_try ()
#10 0x0000555555b47bb3 in unwind::try::h32143614f9ac9c88PGd ()
[...]

Thread 1 (Thread 0x7ffff7fbe780 (LWP 24438)):
#0  pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
#1  0x000055555563fc50 in task::Ops.rt..Runtime::deschedule::h84e263a04c9681b0Nde ()
#2  0x0000555555b48461 in task::Task::deschedule::haa3cdecd71575989pVc ()
#3  0x00005555557598b1 in comm::Receiver$LT$T$GT$::recv_opt::h6035379691551500236 ()
#4  0x00005555556c2a87 in ops::cargo_rustc::compile_targets::h630d332ced3ee7ebyYl ()
#5  0x00005555556b796a in ops::cargo_compile::compile_pkg::h1992b5f7cd947a91Xfh ()
#6  0x00005555556b1ebe in ops::cargo_compile::compile::h1aa0d35ce197f61cZah ()
#7  0x00005555555ce10b in call_main_without_stdin::h2338207437805556094 ()
#8  0x00005555555c09ad in execute::h088c619008f7b816eda ()
#9  0x00005555555bb0d0 in call_main_without_stdin::h6683857340697022421 ()
#10 0x00005555555b935f in main::hcedb2cec117f265f5ca ()
[...]
@alexcrichton
Copy link
Member

I thought I had a specific test to ensure that this didn't happen... Should not so hard to find though!

@alexcrichton alexcrichton added the E-easy Experience: Easy label Nov 11, 2014
@ghost
Copy link

ghost commented Dec 9, 2014

The problem here seems to be, that there actually are checks for cyclic dependencies in place but they are skipped in this case.

The cause is the following section in src/cargo/core/resolver/mod.rs:

260         let my_cx = if early_return {                                                               
261             my_cx                                                                                   
262         } else {
...           ... checks for cyclic dependencies are done here ...

The checks are skipped if early_return is true, which gets set here:

239         let early_return = {
240             my_cx.resolve.graph.link(parent.get_package_id().clone(),
241                                      candidate.get_package_id().clone());                           
242             let prev = match my_cx.activations.entry(key.clone()) {                                 
243                 Occupied(e) => e.into_mut(),
244                 Vacant(e) => e.set(Vec::new()),                                                     
245             }; 
246             if prev.iter().any(|c| c == candidate) {
247                 match cx.resolve.features(candidate.get_package_id()) {                             
248                     Some(prev_features) => {
249                         features.iter().all(|f| prev_features.contains(f))                          
250                     }
251                     None => features.len() == 0,                                                    
252                 }
253             } else {
254                 my_cx.resolve.graph.add(candidate.get_package_id().clone(), &[]);                   
255                 prev.push(candidate.clone());                                                       
256                 false                                                                               
257             }                                                                                       
258         };                                                                                          
259         

As a quick test I tried to just remove the "early return" logic and it seems to do the trick. Cyclic dependencies are reported correctly then.

Unfortunately I do not quite understand what this last segment actually does.
If somebody could give me a hint here, I would be happy to try to find a proper solution.

@ghost
Copy link

ghost commented Jan 6, 2015

I finally had the time to dig a little deeper into this and found that the problem was introduced in 7db89ed.
That commit introduced the early_return flag in the snippet above to prevent recursing into a dependency unless it is activated for the first time or new features are enabled.
Unfortunately the check for cyclic dependencies is only triggered on activation and therefore skipped by this shortcut, too.
So to properly handle this case I guess the check would have to be made alongside the early_return decision.

bors added a commit that referenced this issue Jan 29, 2015
Set out to solve #1236, but ended up solving #834 as well!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Experience: Easy
Projects
None yet
Development

No branches or pull requests

2 participants