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

Editing the Modified (Clock) Time field causes crash #472

Closed
GervaseFT opened this issue Dec 14, 2024 · 18 comments · Fixed by #473
Closed

Editing the Modified (Clock) Time field causes crash #472

GervaseFT opened this issue Dec 14, 2024 · 18 comments · Fixed by #473
Labels
S: triage Issue needs triage.

Comments

@GervaseFT
Copy link

GervaseFT commented Dec 14, 2024

MacOS Ventura 13.2

When I edit the Modified (Clock) Time field, rummage crashes with the following 'stack trace'...

2024-12-14 20:42:51.682 Python[12623:481580] *** Assertion failure in -[NSTextRange initWithLocation:endLocation:], NSTextRange.m:26
Fatal Python error: PyGILState_Release: thread state 0x7f971d804510 must be current when releasing
Python runtime state: initialized

Current thread 0x00007ff85e32f640 (most recent call first):
  File "/Users/glam/Library/Python/3.9/lib/python/site-packages/wx/lib/masked/timectrl.py", line 1214 in SetSelection
  File "/Users/glam/Library/Python/3.9/lib/python/site-packages/wx/lib/masked/textctrl.py", line 140 in _SetSelection
  File "/Users/glam/Library/Python/3.9/lib/python/site-packages/wx/core.py", line 3427 in <lambda>
  File "/Users/glam/Library/Python/3.9/lib/python/site-packages/wx/core.py", line 2262 in MainLoop
  File "/Users/glam/Library/Python/3.9/lib/python/site-packages/rummage/__main__.py", line 55 in run
  File "/Users/glam/Library/Python/3.9/lib/python/site-packages/rummage/__main__.py", line 63 in main
  File "/Users/glam/Library/Python/3.9/bin/rummage", line 8 in <module>
zsh: abort      ~/Library/Python/3.9/bin/rummage
@gir-bot gir-bot added the S: triage Issue needs triage. label Dec 14, 2024
@facelessuser
Copy link
Owner

Can you provide precise steps?

@GervaseFT
Copy link
Author

GervaseFT commented Dec 14, 2024

  1. Start up rummage.
  2. Click on Modified ("Clock") Time field.
  3. Enter a completely different time in the field.
  4. RESULT: Crash.

@facelessuser
Copy link
Owner

I will need at least what version of Rummage you are using and what version of wxPython. If you are not using the latest, an update will likely be required for me to look further as such issues may already be resolved in the latest.

@GervaseFT
Copy link
Author

wxPython 4.2.1

image

@facelessuser
Copy link
Owner

Can you try upgrading wxpython to 4.2.2? This issue is most likely a wxpython issue and not is (but maybe we need to workaround something), so it would be good to see if it exists with the latest wxpython. If it does, I'll try and reproduce when I am in front of my computer.

@GervaseFT
Copy link
Author

For the record, the same Python stack trace also occurs when I try to edit the Created ("Clock") Time field.

It will be some time before I will get the chance to upgrade my wxpython.

@facelessuser
Copy link
Owner

For the record, the same Python stack trace also occurs when I try to edit the Created ("Clock") Time field.

This is expected as they both use wx.lib.masked.TimeCtrl.

I have reproduced the issue, but I suspect that there is a bug in wxPython. This seems to be a new issue. I will see if I can create a simplified example to be sure this is a wxPython issue. If it is, I will still attempt to see if there is some mitigation we can do.

@facelessuser
Copy link
Owner

This problem occurs in wxPython's own TimeCtrl demo, so this is clearly a wxPython issue.

@facelessuser
Copy link
Owner

Here is the linked issue wxWidgets/Phoenix#2657

@facelessuser
Copy link
Owner

I'll see what I can do, but the real fix is going to be wxPython fixing it.

@facelessuser
Copy link
Owner

This issue only affects macOS. I cannot reproduce on any other platform, and as mentioned before, it is an upstream issue. Our options are:

  1. Wait for wxPython to fix it.
  2. Identify some way to get it working on macOS.
  3. Build up a custom time picker control from scratch that does not have this issue. We could retire it at some point when the issue is fixed upstream.

The last option is not ideal as it is a lot of work for something we hope to retire once the real issue is solved. Number 2 is only a good option if we can find a viable path to sidestep this issue. 1 is ideal if, and only if, it is solved quickly and released quickly. We'll have to see what I can come up with.

@facelessuser
Copy link
Owner

@GervaseFT can you let me know if you are on a Intel or Arm (M series) mac?

@facelessuser
Copy link
Owner

While I'm pretty sure this issue is only affecting ARM (M series) macs, I think I have a path forward. We'll likely switch to using https://docs.wxpython.org/wx.adv.TimePickerCtrl.html instead of what we are using now, at least on mac.

IIRC, it is not as configurable, so visually, it may have some slight differences and inconsistencies, but it should work pretty much the same (at least as far as the user is concerned). Internally I'll have to rework some things to support it. I'll have to play around and figure out if it is worth migrating all platforms to it, but at least mac for as long as the other option is broken. Who knows, we may just keep using it from now on as it may fix some other quirks as well.

@facelessuser
Copy link
Owner

So it looks like I have a working solution and it is compatible for Windows and Mac. I still need to test on Linux. Assuming everything checkouts there, I think I'll be able to commit and release. Worst case, I can avoid using the new control for Linux and just apply it to Windows and Mac.

@facelessuser
Copy link
Owner

Fix has been validated on all platforms to work and not cause crashes.

@facelessuser
Copy link
Owner

@GervaseFT I have committed the fix and will be releasing it soon. Please do post back on whether you are on an Intel or ARM (M series) Mac. This way I can report upstream on exactly which machines this affects.

@GervaseFT
Copy link
Author

Please do post back on whether you are on an Intel or ARM (M series) Mac.

Intel

@facelessuser
Copy link
Owner

Oh, interesting. Well, I'll report it upstream. Either way it seems using that time control is just not feasible currently, so we'll have to stick with the new one. It's styling can be inconsistent on Linux, but it doesn't crash.

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

Successfully merging a pull request may close this issue.

3 participants