ripose-jp / Memento

An mpv-based video player for studying Japanese
https://ripose-jp.github.io/Memento/
GNU General Public License v2.0
472 stars 22 forks source link

Segfault when using the new deconjugation matcher #229

Closed Calvin-Xu closed 3 months ago

Calvin-Xu commented 3 months ago

For a minimal reproducible example on my setup, simply scanning the に in

一緒に

triggers a segfault in Memento, as do some other inputs. I wonder if this can be reproduced elsewhere.

On another note, sorry I have not been using Memento lately and only getting around seeing the newer features, both due to other things that cause me to have dropped Anki for almost a year by now and due to mostly graduating from Memento and oftentimes just use a regular player and not on the desktop, which is a bittersweet feeling.

ripose-jp commented 3 months ago

Thanks for reporting this. I can't reproduce this on my setup. If I had to guess you have some dictionary installed that has some sort of data that leads to an out of bounds memory access somewhere in the deconjugator algorithm. Could you give me a list of dictionaries that you have installed and some guidance on where I can find them?

No need to apologize for not using Memento. You've done so much for this project and I'm very grateful. Hearing that you've been able to mostly move on is heartening because in that long term that's point. Good job.

Calvin-Xu commented 3 months ago

Thank you. I have quite a long list of dictionaries:

image

if it helps, I can arrange to send my dictionaries.sqlite in a way that works for you

aecsocket commented 3 months ago

I can confirm this crash happens for me as well with a

(Paused) AV: 00:00:10 / 00:24:11 (1%) A-V:  0.000fish: Job 1, 'memento' terminated by signal SIGSEGV (Address boundary error)

Specifically when using these two enabled dictionaries:

image

And in this text:

忘れたい思い出が染みついた場所だから

Hovering over 思い

image

reliably causes a segfault. The screenshot shows the last frame rendered before it crashed, but I was hovering over the character right before .

Disabling the conjugation matcher in settings seems to avoid the crash, but makes the dictionary a lot less useful (at least to me 😦)

(Side note: these badges in the dictionary popup seem way too big, compared to Yomitan's popup at least)

ripose-jp commented 3 months ago

I was able to reproduce it using some of the aecsocket's dictionaries and his example text. There was a specific condition that could cause partial ordering in the comparator function. I think I've fixed it, and will try to get a new release out as soon as I verify that nothing bad happens to the ordering of terms.

If either of you can reproduce this with the new code. Let me know and feel free to reopen this if it's closed by then.

@aecsocket

(Side note: these badges in the dictionary popup seem way too big, compared to Yomitan's popup at least)

That's a bug in Jitendex as far as I can tell. Just update it and it should fix it. This was also covered in #223.

ripose-jp commented 3 months ago

I don't want to make a new release until I know this stable. Let me know if this solves your issues, it would be a big help.

aecsocket commented 3 months ago

Just built bd830990784b81f80ff4e3d5367c2a9514b910fc and tested out the same text with all dictionaries enabled. It didn't crash this time, so it looks like it's fixed.

Also tried with an updated Jitendex and it fixed the big badges issue.

Calvin-Xu commented 3 months ago

My preliminary usage did not cause any crashes, so it seems fixed.

ripose-jp commented 3 months ago

Thanks everyone's help. I've released v1.4.1 with the fix for this bug.