Repository: servo/servo
File: components/script/script_thread.rs (Rust)

Code before:
        layout_factory: Arc<dyn LayoutFactory>,
        image_cache_factory: Arc<dyn ImageCacheFactory>,
        background_hang_monitor_register: Box<dyn BackgroundHangMonitorRegister>,
    ) -> JoinHandle<()> {
        // Setup pipeline-namespace-installing for all threads in this process.
        // Idempotent in single-process mode.
        PipelineNamespace::set_installer_sender(state.namespace_request_sender.clone());

        let script_thread_id = state.id;
        thread::Builder::new()
            .name(format!("Script#{script_thread_id}"))
            .spawn(move || {
                thread_state::initialize(ThreadState::SCRIPT | ThreadState::LAYOUT);
                PipelineNamespace::install(state.pipeline_namespace_id);
                ScriptEventLoopId::install(state.id);
                let memory_profiler_sender = state.memory_profiler_sender.clone();
                let reporter_name = format!("script-reporter-{script_thread_id:?}");
                let script_thread = Rc::new_cyclic(|weak_script_thread| {
                    ScriptThread::new(
                        state,
                        layout_factory,
                        image_cache_factory,
                        background_hang_monitor_register,
                        weak_script_thread.clone(),
                    )
                });
                SCRIPT_THREAD_ROOT.with(|root| {
                    root.set(Some(Rc::as_ptr(&script_thread)));
                });
                let mut failsafe = ScriptMemoryFailsafe::new(&script_thread);

                // Safety: We ensure that only one JSContext exists in this thread.
                // This is the first one and the only one
                // (minus the ones created when reentering rust code through hooks,
                // but such code was derived from this JSContext so it is okay).
                let mut cx = unsafe { script_thread.get_safe_cx() };

                memory_profiler_sender.run_with_memory_reporting(
                    || script_thread.start(&mut cx),
                    reporter_name,
                    ScriptEventLoopSender::MainThread(script_thread.senders.self_sender.clone()),
                    CommonScriptMsg::CollectReports,
                );

                // This must always be the very last operation performed before the thread completes
                failsafe.neuter();
            })
            .expect("Thread spawning failed")
    }
}


Review comment (suggestion):
Let's have `ScriptThread::new` return the `Rc` instead of passing the handle into the constructor. I mean to move the `Rc::new_cyclic` call to where the structure itself is instantiated.

Code after:
        state: InitialScriptState,
        layout_factory: Arc<dyn LayoutFactory>,
        image_cache_factory: Arc<dyn ImageCacheFactory>,
        background_hang_monitor_register: Box<dyn BackgroundHangMonitorRegister>,
    ) -> JoinHandle<()> {
        // Setup pipeline-namespace-installing for all threads in this process.
        // Idempotent in single-process mode.
        PipelineNamespace::set_installer_sender(state.namespace_request_sender.clone());

        let script_thread_id = state.id;
        thread::Builder::new()
            .name(format!("Script#{script_thread_id}"))
            .spawn(move || {
                thread_state::initialize(ThreadState::SCRIPT | ThreadState::LAYOUT);
                PipelineNamespace::install(state.pipeline_namespace_id);
                ScriptEventLoopId::install(state.id);
                let memory_profiler_sender = state.memory_profiler_sender.clone();
                let reporter_name = format!("script-reporter-{script_thread_id:?}");
                let script_thread = ScriptThread::new(
                    state,
                    layout_factory,
                    image_cache_factory,
                    background_hang_monitor_register,
                );
                SCRIPT_THREAD_ROOT.with(|root| {
                    root.set(Some(Rc::as_ptr(&script_thread)));
                });
                let mut failsafe = ScriptMemoryFailsafe::new(&script_thread);

                // Safety: We ensure that only one JSContext exists in this thread.
                // This is the first one and the only one
                // (minus the ones created when reentering rust code through hooks,
                // but such code was derived from this JSContext so it is okay).
                let mut cx = unsafe { script_thread.get_safe_cx() };

                memory_profiler_sender.run_with_memory_reporting(
                    || script_thread.start(&mut cx),
                    reporter_name,
                    ScriptEventLoopSender::MainThread(script_thread.senders.self_sender.clone()),
                    CommonScriptMsg::CollectReports,
                );

                // This must always be the very last operation performed before the thread completes
                failsafe.neuter();
            })
            .expect("Thread spawning failed")
    }
}

impl ScriptThread {
    pub(crate) fn runtime_handle() -> ParentRuntime {