try to fix issue with ispellchecker on windows
ClosedPublic

Authored by cullmann on Feb 11 2020, 9:20 PM.

Details

Summary

de-init com after release, try to better ensure we check for nullptr

see e.g. backtrace

0 sonnet_ispellchecker.dll ISpellCheckerClient::~ISpellCheckerClient
1 sonnet_ispellchecker.dll ISpellCheckerClient::`scalar deleting destructor'
2 Qt5Core.dll QLibraryPrivate::unload
3 Qt5Core.dll QLibraryStore::cleanup
4 Qt5Core.dll anonymous namespace'::dynamic atexit destructor for 'qlibraryCleanup_dtor_instance_''
5 ucrtbase.dll _lambda_f03950bc5685219e0bcd2087efbe011e_::operator
6 ucrtbase.dll crt_seh_guarded_call_int_::operator
7 ucrtbase.dll _execute_onexit_table
8 Qt5Core.dll dllmain_crt_process_detach
9 Qt5Core.dll dllmain_dispatch
10 ntdll.dll LdrpCallInitRoutine
11 ntdll.dll LdrShutdownProcess
12 ntdll.dll RtlExitUserProcess
13 kernel32.dll FatalExit
14 ucrtbase.dll exit_or_terminate_process
15 ucrtbase.dll common_exit
16 kate.exe
scrt_common_main_seh
17 kernel32.dll BaseThreadInitThunk
18 ntdll.dll RtlUserThreadStart

Diff Detail

Repository
R246 Sonnet
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 22377
Build 22395: arc lint + arc unit
cullmann created this revision.Feb 11 2020, 9:20 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptFeb 11 2020, 9:20 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
cullmann requested review of this revision.Feb 11 2020, 9:20 PM

See crash reports in Windows store.
For me locally, it seemed to work :/

cullmann updated this revision to Diff 75497.Feb 11 2020, 10:04 PM
  • skip com init/deinit

I played a bit with it.
No idea why I get crash reports over the store and can't see this locally.
Given the docs state one shall not do the de-init during unloading but sonnet handles the clients/spellers via some global static that could be a reason...

:=) I think the issue is even much smaller, I just call un-init before release, that is wrong :/
I will quickfix that and avoid other changes (beside the extra nullptr checks).

cullmann updated this revision to Diff 75513.Feb 12 2020, 7:36 AM

de-init com after interface got released, avoids crash

cullmann updated this revision to Diff 75516.Feb 12 2020, 8:35 AM

any cleanup crashs silently

cullmann updated this revision to Diff 75527.Feb 12 2020, 9:27 AM

sonnet cleanup for the dictionaries seems to be too late

try what happen if we leak them, too

cullmann updated this revision to Diff 75532.Feb 12 2020, 9:29 AM

fix argument to create spell checker

cullmann updated this revision to Diff 75534.Feb 12 2020, 9:47 AM

add missing include for windows

cullmann updated this revision to Diff 75536.Feb 12 2020, 9:53 AM

adapt api to used QMap

Ok, tested on Win10 => This solves the issue.

I see no proper way to cleanup this without thinking more about how sonnet does cache the dictionaries internally.
It just cleans up "too late" with a global static.

Given sonnet anyways will keep all stuff loaded until program exit, doing the same in the backend doesn't look that bad for me as an workaround.

I tried it with

https://binary-factory.kde.org/job/Kate_Release_win64/782/

> this is the first version that doesn't always crash ;=)

vonreth accepted this revision.Feb 13 2020, 10:24 AM

looks good but in a future version of sonnet plugins should be unloaded when shutting down and we should properly unload stuff.

This revision is now accepted and ready to land.Feb 13 2020, 10:24 AM
This revision was automatically updated to reflect the committed changes.