ICE: refcell panic in rustdoc on facade crate

[aturon]

I got the following ICE when running cargo doc on a facade crate that re-exports various types that use intra-rustdoc links:

thread 'rustc' panicked at 'already borrowed: BorrowMutError', libcore/result.rs:945:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
   1: std::sys_common::backtrace::_print
   2: std::panicking::default_hook::{{closure}}
   3: std::panicking::default_hook
   4: std::panicking::rust_panic_with_hook
   5: std::panicking::begin_panic
   6: std::panicking::begin_panic_fmt
   7: rust_begin_unwind
   8: core::panicking::panic_fmt
   9: core::result::unwrap_failed
  10: rustdoc::clean::inline::record_extern_trait
  11: rustdoc::clean::register_def
  12: <[syntax::ast::Attribute] as rustdoc::clean::Clean<rustdoc::clean::Attributes>>::clean
  13: <rustc::ty::AssociatedItem as rustdoc::clean::Clean<rustdoc::clean::Item>>::clean
  14: <alloc::vec::Vec<T> as alloc::vec::SpecExtend<T, I>>::spec_extend
  15: rustdoc::clean::inline::build_external_trait
  16: rustdoc::clean::inline::record_extern_trait
  17: rustdoc::clean::inline::build_impl
  18: rustdoc::clean::inline::build_impls
  19: rustdoc::clean::inline::try_inline
  20: <rustdoc::doctree::Import as rustdoc::clean::Clean<alloc::vec::Vec<rustdoc::clean::Item>>>::clean
  21: <rustdoc::doctree::Module as rustdoc::clean::Clean<rustdoc::clean::Item>>::clean
  22: <rustdoc::visit_ast::RustdocVisitor<'a, 'tcx, 'rcx> as rustdoc::clean::Clean<rustdoc::clean::Crate>>::clean
  23: rustdoc::core::run_core::{{closure}}
  24: rustc::ty::context::TyCtxt::create_and_enter
  25: rustdoc::core::run_core

[aturon]

cc @QuietMisdreavus @Manishearth

[QuietMisdreavus]

Panic occurs because the recursive call here still holds the borrow for the external_traits RefCell, but it apparently needs to re-enter? Let me take a look.

rust/src/librustdoc/clean/inline.rs

Lines 496 to 500 in 27a046e

	pub fn record_extern_trait(cx: &DocContext, did: DefId) {
	cx.external_traits.borrow_mut().entry(did).or_insert_with(|| {
	build_external_trait(cx, did)
	});
	}

[QuietMisdreavus]

PM 052135 <misdreavus> oh, i see
PM 052143 <misdreavus> the docs are referring to a trait that hasn't been loaded yet
PM 052320 <misdreavus> when the trait is local, this is fine, because all that info is still in quick access
PM 052332 <misdreavus> but because the trait is remote, we need to pull it in
PM 052356 <misdreavus> but! we were already pulling in a remote trait
PM 052501 <misdreavus> clean::inline::record_extern_trait, farther up the stack, is waiting to insert a clean::Trait into external_traits
PM 052552 <misdreavus> so now i need to manually release that borrow once i know that i need to calculate it

The relevant culprit for intra-links is this line right here:

rust/src/librustdoc/clean/mod.rs

Line 1169 in 27a046e

let id = register_def(cx, def);

Because it kicks off the second record_extern_trait call.

[Manishearth]

We can also not use the entry API and do it manually?

[QuietMisdreavus]

That's my thought; just check for the presence of the key, bail early, then build the external trait without holding a borrow on cx.external_traits. I'm building right now.

[Manishearth]

I have a build already, want me to do it?

[Manishearth]

This doesn't work because this leads to a stack overflow where we recursively keep trying to register the def. We need to buffer it up or something.

[QuietMisdreavus]

#48415