Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Seg-Fault fix #75

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Seg-Fault fix #75

wants to merge 4 commits into from

Conversation

jweezy24
Copy link

This is the code in reference that fixes the seg-fault from issue 72. This works on base Arch Linux with python3.12. I have not tested it on other operating systems.

@kaixxx
Copy link
Owner

kaixxx commented Jul 23, 2024

Thanks, @jweezy24! I will look into this in a couple of days.

@kaixxx
Copy link
Owner

kaixxx commented Aug 7, 2024

Hi @jweezy24
I'm trying to get my head around this. As far as I can tell, there are two issues addressed here:

  1. Identifying the right python binary ('python' or 'python3'). This seems perfectly fine to me and would make noScribe more robust for people running it from source (and having python 2 installed on their system). I'm happy to accept this change.

  2. Preventing the segfault happening while writing to log_textbox. Here, your solution seems a bit complicated to me and also has some issues.

  • First, if logging to log_textbox fails, the function should not simply return but at least try to write the message to the log file (if where == both).
  • You check quite early if the log_textbox exists (self.log_textbox.winfo_exists()). Shouldn’t this be all that it needs to prevent the whole segfault issue? In this case, we could simply use the old log function and just add an additional condition to if where != 'file', like: if where != 'file' and self.log_textbox.winfo_exists(). This would also ensure that the error is written to the log file even if logging on screen fails.
  • Finally, regarding error handling: Since most people use the compiled version of noScribe without a terminal window, print statements are not very helpful to identify errors. All errors should be logged using the log function. This is a little complicated here because if the log function itself causes the error, we risk running into an endless recursion (that is, the python recursion limit of 1000). One way to lower the risk of this happening would be to log errors that happen while writing to log_textbox only to the log file (with the flag where='file').

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants