Fix unloading of C plugins

[lluchs]

76b5d84 removed the direct call to self.libpanda.panda_unload_plugins(), introduced by bb83a2f. As noted in the comment in unload_plugins() in that commit, that function doesn't unload C plugins during shutdown, so the direct call is still required.

(Or at least I assume that is what is happening - adding the call back fixes uninit calls for me anyways.)

[AndrewFasano]

I think this might be a bit more complicated than it seems at first. Let me first walk through what we want here (mostly to have this written down for myself, seems like you might already understand this):

When PANDA goes to terminate an emulation because

• The guest has powered off
• A user has typed quit into the monitor
• A C/C++ plugin has requested to terminate the guest with a call to panda_stop_vm, or
• A Python/Rust plugin has requested to terminate the guest with a call to end_analysis (which then calls stop_run)

We want to inform all the loaded plugins of the imminent shutdown by:

• Calling the unint_plugin method in C plugins
• Calling the unit method in Python plugins
• (Maybe?) Call some shutdown method in Rust plugins

And finally clean up/unload plugin state, terminate the emulation, and exit.

In #1313 (76b5d84) we fixed the case where a shutdown triggered from C/C++ wouldn't cause the unint method of pyplugins to run.

It sounds like the issue you're seeing is that the uninit_plugin method in C plugins isn't being run if a Python plugin shuts the emulator down - is that correct?

I'm worried that there might be some race conditions or issues with the change you have here because the existing unload_plugins method already calls libpanda.panda_unload_plugins - it queues up logic to unload pyplugins and c plugins at the next main_loop_wait. The main_loop_wait method exists to avoid deadlocks by scheduling certain work to occur in a time that we know to be (somewhat) "safe". If we unload plugins at other times I think we have a risk of unloading code while it might still be called and this could cause crashes during shutdown.

panda/panda/python/core/pandare/panda.py

Lines 781 to 785 in 3787038

	# In next main loop wait, unload all python plugin
	self.queue_main_loop_wait_fn(self._unload_pyplugins)
	# Then unload C plugins. May be unsafe to do except from the top of the main loop (taint segfaults otherwise)
	self.queue_main_loop_wait_fn(self.libpanda.panda_unload_plugins)

But there's a comment specifically about this method being used during shutdown:

    XXX: If called during shutdown/exit, c plugins won't be unloaded
   because the next main_loop_wait will never happen. Instead, call
   panda.panda_finish directly (which is done at the end of panda.run())

Tracking that down, I see the end of the python logic driving emulation calls panda.panda_finish which ultimately returns execution to shutdown logic in vl.c here - but I don't see anything unloading plugins on this path - perhaps we should add this unload logic there instead?

panda/vl.c

Lines 5088 to 5115 in 3787038

	PANDA_MAIN_FINISH:
	if(rr_in_record()){
	rr_do_end_record();
	}
	int x;
	for(x=0; x < gargc ; ++x )
	{
	free(gargv[x]);
	gargv[x] = NULL;
	}
	free(gargv);
	replay_disable_events();
	iothread_stop_all();
	panda_cleanup();
	pause_all_vcpus();
	bdrv_close_all();
	res_free();
	/* vhost-user must be cleaned up before chardevs. */
	net_cleanup();
	audio_cleanup();
	monitor_cleanup();
	qemu_chr_cleanup();

[lluchs]

Thanks for the quick and detailed response! I read a bit more source code and now I'm less sure how everything is supposed to work :)

My Python script loads a Rust plugin and calls panda.run() and panda.stop_run() a couple times. It's important that uninit runs in the Rust plugin so that it flushes its output.

If I understand what is happening correctly, the call to stop_run() unloads the plugins via what happens in run(). It's not clear to me if that is what is supposed to happen - the Python API documentation says that end_analysis() will unload the plugins, but stop_run() will not.

Your idea with unloading the plugins in vl.c is actually what is already happening. It's just hidden away in the call to panda_cleanup():

panda/panda/src/common.c

Lines 229 to 235 in 3787038

	void panda_cleanup(void) {
	// PANDA: unload plugins
	panda_unload_plugins();
	if (pandalog) {
	pandalog_cc_close();
	}
	}

However, that code actually never runs if you don't call panda_finish() yourself. The comment cited above implies that such a call is supposed to be at the end of panda.run(), but that is not the case. (I didn't check whether it used to be there.)

So now I'm not sure what to do here. My code now works after adding a call to panda_finish(). So possibly this is more a documentation issue and all the examples should have such a call (rather than just calling end_analysis() or stop_run()). Or end_analysis() should somehow call panda_finish() (but probably not stop_run(), since that is actually not supposed to unload plugins?).