search

mu-editor / mu

A small, simple editor for beginner Python programmers. Written in Python and Qt5.
http://codewith.mu
GNU General Public License v3.0
1.41k stars 435 forks source link

fix 2375 - this seems to fix the issue I ran into #2376

Closed haikusw closed 1 year ago

haikusw commented 1 year ago

Reading the documentation, it appears that unlock()'ing the shared memory is not sufficient and that one shoulddetach()` it also. Adding that seems to have resolved this issue #2375 on macOS 10.13.6 with python 3.8.16.

tjguk commented 1 year ago

Well thank you. Not sure how I missed that rather obvious point! (Nor why it worked in all my tests...). Let me take your PR for a spin then hopefully I can merge in short order.

haikusw commented 1 year ago

It appears that some operating systems may detach automatically? Not 100% sure. And it doesn't solve the cash of crashing not detaching, obviously.

haikusw commented 1 year ago

I'm definitely not at all familiar with Qt nor done very much python, so please be skeptical :)

tjguk commented 1 year ago

In short (and as expected) it runs fine on my local machine. I've just enabled the GH checks for your branch -- from experience the RPi containers will take a while to run. But assuming everything runs clear on those platforms I'm happy to merge.

haikusw commented 1 year ago

Thanks. I have to get some sleep (~2am!) but tomorrow I'll see about updating my branch for #2315 from master after you've merged this and then making a PR for it.

tjguk commented 1 year ago

@haikusw - All tests cleared on live. 👍

I had a quick look at the PR you'd recently closed against the #2315 issue. In principle it looks ok, but I'd like to get the view of the other devs as there may be experience from elsewhere which tells against the approach you're suggesting.

haikusw commented 1 year ago

@tjguk unfortunately, this fix just disabled the check for already running because I didn't understand how __exit__ worked. I apologize. I came back to see about handling the app crash hole and did more manual testing.

The automated tests didn't catch this.

I've been looking into how to fix this bug #2375 properly and I'm not finding the right way to do it.

As I understand it, the code needs to call _shared_memory.detach() on both normal exit and crash paths to avoid this #2375 issue.

Anyway, I apologize that my ignorance of python created a non-fix. I'll keep looking for the right way to do it; suggestions welcome.

haikusw commented 1 year ago

New draft PR #2386 works on this macOS 10.13.6 machine with python 3.8.16, but one test is failing in CI on a bunch of different configurations.