platomav
Fix segfault on exit when update check is enabled + small fixes
Hi, Thanks for maintaining this tool. I ran into a few issues lately so I'm sharing my fixes:
-
Make MCE.py executable
-
Fix crash when stdout is not a TTY on Linux
Without this change, the following happened when piping/redirecting the output of MCE.py:
Error: MC Extractor v1.102.0 crashed, please report the following: Traceback (most recent call last): File "/root/MCExtractor/MCE.py", line 1093, in <module> elif sys_os.startswith('linux') or sys_os == 'darwin' or sys_os.find('bsd') != -1 : sys.stdout.write('\x1b]2;' + mce_title + '\x07') ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3/dist-packages/colorama/ansitowin32.py", line 47, in write self.__convertor.write(text) File "/usr/lib/python3/dist-packages/colorama/ansitowin32.py", line 177, in write self.write_and_convert(text) File "/usr/lib/python3/dist-packages/colorama/ansitowin32.py", line 199, in write_and_convert text = self.convert_osc(text) ^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3/dist-packages/colorama/ansitowin32.py", line 272, in convert_osc winterm.set_title(params[1]) ^^^^^^^^^^^^^^^^^ AttributeError: 'NoneType' object has no attribute 'set_title'This looks like a colorama bug, see https://github.com/tartley/colorama/issues/209.
The workaround is to skip the title change if stdout is not a TTY.
-
Fix segfault on exit when update check is enabled
Before this patch, the script could often crash with segfaults on Debian 12/Python 3.11. It apparently when processing small files. In that case, there was likely a race condition where Python tried to terminate the update thread while it was already dead.
It was easy to reproduce with:
for i in {1..100}; do ./MCE.py -skip -exit /lib/firmware/amd-ucode/microcode_amd_fam16h.bin >& /dev/null || echo failed at iteration $i doneGDB showed:
Thread 1 "python3" received signal SIGABRT, Aborted. __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44 44 ./nptl/pthread_kill.c: No such file or directory. [Thread debugging using libthread_db enabled]Using multiprocessing with daemon=True, there are no more crashes. Processes are probably easier to kill properly. The result of the update check is now shared using a queue.
Hello,
Thank you for the PR! I had also noticed the thread crash on Windows, when I would execute and terminate the utility fast enough.
Generally, MCE will be re-written from scratch at some point, so all of these issues will go away. But, for now, merging this can help.
I would like to request one thing, to reverse the whitespace changes only. I hate them, and obviously these won't be there when the project is re-written with good code practices, but for now I'd like to avoid a PR which touches every line of the code more or less, due to padding.
Generally, MCE will be re-written from scratch at some point
Cool :) I wanted to make pltable and colorama optional for environments where only stdlib packages are available but pltable really is used by a lot of the code. It would be awesome if the parser classes could be used separately without running the whole script, for instance to display what microcodes are present within a blob.
I dropped the whitespace commit.
Yeah, colorama will be removed (no longer maintained) and probably the tabloid-style text too, will see if I do anything similar but as you said, the basic parsing etc takes precedence with these types of refactors. It will be re-written as a project with proper imports, classes, requirements etc.
Thank you for the whitespace drop. I tried to execute MCE.py (no parameters) and it crashes at the "main menu" (also something that will go away - anyway):
╔════════════════════════════════════════════╗
║ MC Extractor v1.102.0 r329 ║
╚════════════════════════════════════════════╝
Welcome to Intel, AMD, VIA & Freescale Microcode Extractor
Input a file name/path or press Enter to list options
File: None
Option(s):
Error: MC Extractor v1.102.0 crashed, please report the following:
Traceback (most recent call last):
File "<string>", line 1, in <module>
from multiprocessing.spawn import spawn_main; spawn_main(parent_pid=10768, pipe_handle=488)
~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Program Files\Python313\Lib\multiprocessing\spawn.py", line 122, in spawn_main
exitcode = _main(fd, parent_sentinel)
File "C:\Program Files\Python313\Lib\multiprocessing\spawn.py", line 131, in _main
prepare(preparation_data)
~~~~~~~^^^^^^^^^^^^^^^^^^
File "C:\Program Files\Python313\Lib\multiprocessing\spawn.py", line 246, in prepare
_fixup_main_from_path(data['init_main_from_path'])
~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Program Files\Python313\Lib\multiprocessing\spawn.py", line 297, in _fixup_main_from_path
main_content = runpy.run_path(main_path,
run_name="__mp_main__")
File "<frozen runpy>", line 287, in run_path
File "<frozen runpy>", line 98, in _run_module_code
File "<frozen runpy>", line 88, in _run_code
File "C:\Users\plato\Desktop\MCExtractor-fix_segfaults\MCE.py", line 1032, in <module>
if not param.upd_dis : update_process.start() # Start as soon as possible (mce_dir, db_path)
~~~~~~~~~~~~~~~~~~~~^^
File "C:\Program Files\Python313\Lib\multiprocessing\process.py", line 121, in start
self._popen = self._Popen(self)
~~~~~~~~~~~^^^^^^
File "C:\Program Files\Python313\Lib\multiprocessing\context.py", line 224, in _Popen
return _default_context.get_context().Process._Popen(process_obj)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^
File "C:\Program Files\Python313\Lib\multiprocessing\context.py", line 337, in _Popen
return Popen(process_obj)
File "C:\Program Files\Python313\Lib\multiprocessing\popen_spawn_win32.py", line 47, in __init__
prep_data = spawn.get_preparation_data(process_obj._name)
File "C:\Program Files\Python313\Lib\multiprocessing\spawn.py", line 164, in get_preparation_data
_check_not_importing_main()
~~~~~~~~~~~~~~~~~~~~~~~~~^^
File "C:\Program Files\Python313\Lib\multiprocessing\spawn.py", line 140, in _check_not_importing_main
raise RuntimeError('''
...<16 lines>...
''')
RuntimeError:
An attempt has been made to start a new process before the
current process has finished its bootstrapping phase.
This probably means that you are not using fork to start your
child processes and you have forgotten to use the proper idiom
in the main module:
if __name__ == '__main__':
freeze_support()
...
The "freeze_support()" line can be omitted if the program
is not going to be frozen to produce an executable.
To fix this issue, refer to the "Safe importing of main module"
section in https://docs.python.org/3/library/multiprocessing.html
Sorry, I didn't test on Windows. I can add the if __name__ == '__main__' code as mentioned in https://docs.python.org/3/library/multiprocessing.html#the-spawn-and-forkserver-start-methods to avoid this but it'll change a lot of lines, do you mind?
Thank you for the offer. Due to the (terrible) way it's written now, it is not really "__main__" compatible, and such change would indeed require too many changes. There is no point really, it is going to be re-written either way. Btw, the update check functionality with the thread etc has already been removed in the draft of the refactor so, unless the segfault is a big issue, maybe we can leave it as it was. If it is an important issue, I suggest to use -duc and if that does not help either, I prefer to simply delete the update check code altogether.
If it is an important issue, I suggest to use -duc
Yes, I already do that :) It took me several hours to understand why the script was crashing so I thought I'd share it but if it's going to be rewritten, there is no hurry. Out of curiosity, if you call freeze_support(), does it make the script work on Windows again or is that unrelated?
Yes no problem, glad to hear that at least -duc is helping. I agree that we can wait until that thing is re-written. I did not test that suggestion from the message (although I suspect it's more complex than that - looks like a generic tip - we are not even freezing anything). The script does not really have a "main" to put this line in, it is effectively some top level code followed by an enormous "for" loop. Trust me, it is terrible haha. 😅