sergey-dryabzhinsky / qt-webkit-kiosk

This is simple browser application written on Qt & QtWebkit.
57 stars 37 forks source link

qt-webkit-kiosk fails to close #37

Closed HED-jzignego closed 2 years ago

HED-jzignego commented 2 years ago

When running the browser, I start it with start-stop-daemon in an init script. Sometimes I want to shut it down shortly after starting it, however about 10% of the time it fails to stop. I've enabled debugging output and I found that: this runs: https://github.com/sergey-dryabzhinsky/qt-webkit-kiosk/blob/master/src/unixsignals.cpp#L107-L112

But the other end never sees it. So this never runs: https://github.com/sergey-dryabzhinsky/qt-webkit-kiosk/blob/master/src/unixsignals.cpp#L114-L135

I can't seem to reproduce the issue on my desktop, only on my embedded device unfortunately.

So I found this documentation here: https://doc.qt.io/qt-5/unix-signals.html

My question is thus: would you be willing to accept a PR that replaces most of unixsignals.cpp and socketpair.cpp with the simpler setup of just using SocketNotifier? I will write the PR if you are willing to accept it.

This would eliminate the need for most of the code in both of those classes.

sergey-dryabzhinsky commented 2 years ago

Feel free to make a PR.

вт, 15 февр. 2022 г. в 01:03, HED-jzignego @.***>:

When running the browser, I start it with start-stop-daemon in an init script. Sometimes I want to shut it down shortly after starting it, however about 10% of the time it fails to stop. I've enabled debugging output and I found that: this runs:

https://github.com/sergey-dryabzhinsky/qt-webkit-kiosk/blob/master/src/unixsignals.cpp#L107-L112

But the other end never sees it. So this never runs:

https://github.com/sergey-dryabzhinsky/qt-webkit-kiosk/blob/master/src/unixsignals.cpp#L114-L135

So I found this documentation here: https://doc.qt.io/qt-5/unix-signals.html

My question is thus: would you be willing to accept a PR that replaces most of unixsignals.cpp and socketpair.cpp with the simpler setup of just using SocketNotifier? I will write the PR if you are willing to accept it.

This would eliminate the need for most of the code in both of those classes.

— Reply to this email directly, view it on GitHub https://github.com/sergey-dryabzhinsky/qt-webkit-kiosk/issues/37, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFPGHMTELPSVQ3INJO2R4LU3F34HANCNFSM5OMWVJRA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>

HED-jzignego commented 2 years ago

Ok I will do that, can you merge this PR first: https://github.com/sergey-dryabzhinsky/qt-webkit-kiosk/pull/38, that does some prep work to get it ready for the real work.

HED-jzignego commented 2 years ago

I do have one question, the approach in https://doc.qt.io/qt-5/unix-signals.html uses AF_UNIX sockets, those are now supported in windows 10, is that good enough for windows support or do you want to support older versions of windows too even though they no longer are receiving security updates?