skr0x1c0 / ipybinja

IPython console widget for Binary Ninja
MIT License
12 stars 0 forks source link

crash on plugin-manager installs #2

Closed psifertex closed 1 year ago

psifertex commented 1 year ago

Describe the bug Installing the plugin on MacOS causes a crash on next BN restart.

Installation appeared to work normally but the plugin causes a crash when trying to run:

Fatal Python error: PyThreadState_Get: the function must be called with the GIL held, but the GIL is released (the current Python thread state is NULL)
Python runtime state: finalizing (tstate=0x00000001462044c0)

fish: Job 1, '/Applications/Binary\ Ninja-dev…' terminated by signal SIGABRT (Abort)

To Reproduce Steps to reproduce the behavior:

  1. Install via plugin manager
  2. Use the "install ipython kernel" action
  3. Restart BN

Expected behavior It should not crash.

Environment (please complete the following information):

Additional context Debug logging:

[0:8804117752360163501 Core debug] Path:
[0:8502617344 Default error] [IPKernelApp] CRITICAL | Bad config encountered during initialization: The 'kernel_class' trait of <ipykernel.kernelapp.IPKernelApp object at 0x14ea0cd30> instance must be a type, but 'ipybinja.ThreadedKernel' could not be imported
Fatal Python error: PyThreadState_Get: the function must be called with the GIL held, but the GIL is released (the current Python thread state is NULL)
Python runtime state: finalizing (tstate=0x0000000121f77e80)

fish: Job 1, '/Applications/Binary\ Ninja-dev…' terminated by signal SIGABRT (Abort)
psifertex commented 1 year ago

Ahh, I bet I know the issue from the debug log.

When you load a plugin via the plugin manager, the namespace changes unfortunately. There should probably be a try / catch around any import ipybinja that should also check for skr0x1c0_ipybinja as the module name.

It's a kinda annoying quirk of the plugin manager installation that I'd like to be able to change but haven't had time to refactor.

psifertex commented 1 year ago

If you want to test releases within the plugin manager ecosystem you can actually setup a test repository and add it via pluginManager.unofficialUrl and pluginManager.unofficialName. You can see how I was testing one here: https://github.com/Vector35/community-plugins/tree/test

Cloning that branch into a separate repo should let you test it more easily without doing updates to the main plugin manager. I'm going to remove from the main BN plugin manager temporarily until the crash is resolved.

skr0x1c0 commented 1 year ago

Hello Jordan, thanks for the report and tips. This issue is fixed in commit 3a8f2a71133af174bd5f9a4be5c32c28083a1c7c