Message ID | 20250815141237.2235085-1-barnabas.pocze@ideasonboard.com |
---|---|
State | New |
Headers | show |
Series |
|
Related | show |
Hi Barnabás, Thank you for the patch. On Fri, Aug 15, 2025 at 04:12:37PM +0200, Barnabás Pőcze wrote: > Use a signalfd to detect `SIGINT` instead of registering a signal handler in > order to remove the `CamApp` singleton. This is better than using a normal > signal handler: a signal can arrive after the `CamApp` object has been destroyed, > leading to a use-after-free. Modifying the `CamApp::app_` in the destructor is > not a good alternative because that would not be async signal safe (unless it > is made atomic). While this is true, it's also not really much of an issue in my opinion, or at least a very minor one. Now that the patch exists lets get it merged, but you don't have to hunt for these kind of problems in cam or qcam :-) > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/apps/cam/main.cpp | 66 +++++++++++++++++++------------------------ > 1 file changed, 29 insertions(+), 37 deletions(-) > > diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp > index fa266eca6..cbc85b59f 100644 > --- a/src/apps/cam/main.cpp > +++ b/src/apps/cam/main.cpp > @@ -13,6 +13,8 @@ > #include <signal.h> > #include <string.h> > > +#include <sys/signalfd.h> You can keep this grouped with the previous headers. > + > #include <libcamera/libcamera.h> > #include <libcamera/property_ids.h> > > @@ -27,15 +29,12 @@ using namespace libcamera; > class CamApp > { > public: > - CamApp(); > - > - static CamApp *instance(); > + CamApp(EventLoop &loop); > > int init(int argc, char **argv); > void cleanup(); > > int exec(); > - void quit(); > > private: > void cameraAdded(std::shared_ptr<Camera> cam); > @@ -46,26 +45,17 @@ private: > > static std::string cameraName(const Camera *camera); > > - static CamApp *app_; > OptionsParser::Options options_; > > std::unique_ptr<CameraManager> cm_; > > std::atomic_uint loopUsers_; > - EventLoop loop_; > + EventLoop *loop_ = nullptr; > }; > > -CamApp *CamApp::app_ = nullptr; > - > -CamApp::CamApp() > - : loopUsers_(0) > -{ > - CamApp::app_ = this; > -} > - > -CamApp *CamApp::instance() > +CamApp::CamApp(EventLoop &loop) > + : loopUsers_(0), loop_(&loop) > { > - return CamApp::app_; > } > > int CamApp::init(int argc, char **argv) > @@ -103,11 +93,6 @@ int CamApp::exec() > return ret; > } > > -void CamApp::quit() > -{ > - loop_.exit(); > -} > - > int CamApp::parseOptions(int argc, char *argv[]) > { > StreamKeyValueParser streamKeyValue; > @@ -205,7 +190,7 @@ void CamApp::cameraRemoved(std::shared_ptr<Camera> cam) > void CamApp::captureDone() > { > if (--loopUsers_ == 0) > - EventLoop::instance()->exit(0); > + loop_->exit(0); > } > > int CamApp::run() > @@ -290,7 +275,7 @@ int CamApp::run() > } > > if (loopUsers_) > - loop_.exec(); > + loop_->exec(); > > /* 6. Stop capture. */ > for (const auto &session : sessions) { > @@ -345,29 +330,36 @@ std::string CamApp::cameraName(const Camera *camera) > return name; > } > > -namespace { > - > -void signalHandler([[maybe_unused]] int signal) > +int main(int argc, char **argv) > { > - std::cout << "Exiting" << std::endl; > - CamApp::instance()->quit(); > -} > + auto sigfd = [] { > + sigset_t ss; > > -} /* namespace */ > + sigemptyset(&ss); > + sigaddset(&ss, SIGINT); > + sigprocmask(SIG_BLOCK, &ss, nullptr); > > -int main(int argc, char **argv) > -{ > - CamApp app; > + return UniqueFD(signalfd(-1, &ss, SFD_CLOEXEC | SFD_NONBLOCK)); > + }(); Why a lambda ? > + if (!sigfd.isValid()) > + return EXIT_FAILURE; > + > + EventLoop loop; > + CamApp app(loop); > int ret; > > + loop.addFdEvent(sigfd.get(), EventLoop::Read, [&] { > + signalfd_siginfo si; > + std::ignore = read(sigfd.get(), &si, sizeof(si)); > + > + std::cout << "Exiting" << std::endl; > + loop.exit(0); > + }); It could make things simpler to handle this in the CamApp class, you won't have to change where the EventLoop is instantiated. > + > ret = app.init(argc, argv); > if (ret) > return ret == -EINTR ? 0 : EXIT_FAILURE; > > - struct sigaction sa = {}; > - sa.sa_handler = &signalHandler; > - sigaction(SIGINT, &sa, nullptr); > - > if (app.exec()) > return EXIT_FAILURE; >
Hi 2025. 08. 16. 0:21 keltezéssel, Laurent Pinchart írta: > Hi Barnabás, > > Thank you for the patch. > > On Fri, Aug 15, 2025 at 04:12:37PM +0200, Barnabás Pőcze wrote: >> Use a signalfd to detect `SIGINT` instead of registering a signal handler in >> order to remove the `CamApp` singleton. This is better than using a normal >> signal handler: a signal can arrive after the `CamApp` object has been destroyed, >> leading to a use-after-free. Modifying the `CamApp::app_` in the destructor is >> not a good alternative because that would not be async signal safe (unless it >> is made atomic). > > While this is true, it's also not really much of an issue in my opinion, > or at least a very minor one. Now that the patch exists lets get it > merged, but you don't have to hunt for these kind of problems in cam or > qcam :-) I actually did run into this on multiple occasions. :( > >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> --- >> src/apps/cam/main.cpp | 66 +++++++++++++++++++------------------------ >> 1 file changed, 29 insertions(+), 37 deletions(-) >> >> diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp >> index fa266eca6..cbc85b59f 100644 >> --- a/src/apps/cam/main.cpp >> +++ b/src/apps/cam/main.cpp >> @@ -13,6 +13,8 @@ >> #include <signal.h> >> #include <string.h> >> >> +#include <sys/signalfd.h> > > You can keep this grouped with the previous headers. > >> + >> #include <libcamera/libcamera.h> >> #include <libcamera/property_ids.h> >> >> @@ -27,15 +29,12 @@ using namespace libcamera; >> class CamApp >> { >> public: >> - CamApp(); >> - >> - static CamApp *instance(); >> + CamApp(EventLoop &loop); >> >> int init(int argc, char **argv); >> void cleanup(); >> >> int exec(); >> - void quit(); >> >> private: >> void cameraAdded(std::shared_ptr<Camera> cam); >> @@ -46,26 +45,17 @@ private: >> >> static std::string cameraName(const Camera *camera); >> >> - static CamApp *app_; >> OptionsParser::Options options_; >> >> std::unique_ptr<CameraManager> cm_; >> >> std::atomic_uint loopUsers_; >> - EventLoop loop_; >> + EventLoop *loop_ = nullptr; >> }; >> >> -CamApp *CamApp::app_ = nullptr; >> - >> -CamApp::CamApp() >> - : loopUsers_(0) >> -{ >> - CamApp::app_ = this; >> -} >> - >> -CamApp *CamApp::instance() >> +CamApp::CamApp(EventLoop &loop) >> + : loopUsers_(0), loop_(&loop) >> { >> - return CamApp::app_; >> } >> >> int CamApp::init(int argc, char **argv) >> @@ -103,11 +93,6 @@ int CamApp::exec() >> return ret; >> } >> >> -void CamApp::quit() >> -{ >> - loop_.exit(); >> -} >> - >> int CamApp::parseOptions(int argc, char *argv[]) >> { >> StreamKeyValueParser streamKeyValue; >> @@ -205,7 +190,7 @@ void CamApp::cameraRemoved(std::shared_ptr<Camera> cam) >> void CamApp::captureDone() >> { >> if (--loopUsers_ == 0) >> - EventLoop::instance()->exit(0); >> + loop_->exit(0); >> } >> >> int CamApp::run() >> @@ -290,7 +275,7 @@ int CamApp::run() >> } >> >> if (loopUsers_) >> - loop_.exec(); >> + loop_->exec(); >> >> /* 6. Stop capture. */ >> for (const auto &session : sessions) { >> @@ -345,29 +330,36 @@ std::string CamApp::cameraName(const Camera *camera) >> return name; >> } >> >> -namespace { >> - >> -void signalHandler([[maybe_unused]] int signal) >> +int main(int argc, char **argv) >> { >> - std::cout << "Exiting" << std::endl; >> - CamApp::instance()->quit(); >> -} >> + auto sigfd = [] { >> + sigset_t ss; >> >> -} /* namespace */ >> + sigemptyset(&ss); >> + sigaddset(&ss, SIGINT); >> + sigprocmask(SIG_BLOCK, &ss, nullptr); >> >> -int main(int argc, char **argv) >> -{ >> - CamApp app; >> + return UniqueFD(signalfd(-1, &ss, SFD_CLOEXEC | SFD_NONBLOCK)); >> + }(); > > Why a lambda ? > >> + if (!sigfd.isValid()) >> + return EXIT_FAILURE; >> + >> + EventLoop loop; >> + CamApp app(loop); >> int ret; >> >> + loop.addFdEvent(sigfd.get(), EventLoop::Read, [&] { >> + signalfd_siginfo si; >> + std::ignore = read(sigfd.get(), &si, sizeof(si)); >> + >> + std::cout << "Exiting" << std::endl; >> + loop.exit(0); >> + }); > > It could make things simpler to handle this in the CamApp class, you > won't have to change where the EventLoop is instantiated. > >> + >> ret = app.init(argc, argv); >> if (ret) >> return ret == -EINTR ? 0 : EXIT_FAILURE; >> >> - struct sigaction sa = {}; >> - sa.sa_handler = &signalHandler; >> - sigaction(SIGINT, &sa, nullptr); >> - >> if (app.exec()) >> return EXIT_FAILURE; >>
On Sat, Aug 16, 2025 at 12:28:49AM +0200, Barnabás Pőcze wrote: > 2025. 08. 16. 0:21 keltezéssel, Laurent Pinchart írta: > > On Fri, Aug 15, 2025 at 04:12:37PM +0200, Barnabás Pőcze wrote: > >> Use a signalfd to detect `SIGINT` instead of registering a signal handler in > >> order to remove the `CamApp` singleton. This is better than using a normal > >> signal handler: a signal can arrive after the `CamApp` object has been destroyed, > >> leading to a use-after-free. Modifying the `CamApp::app_` in the destructor is > >> not a good alternative because that would not be async signal safe (unless it > >> is made atomic). > > > > While this is true, it's also not really much of an issue in my opinion, > > or at least a very minor one. Now that the patch exists lets get it > > merged, but you don't have to hunt for these kind of problems in cam or > > qcam :-) > > I actually did run into this on multiple occasions. :( Do you mean crashes because a signal arrived when the application was closing ? How did that happen ? > >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > >> --- > >> src/apps/cam/main.cpp | 66 +++++++++++++++++++------------------------ > >> 1 file changed, 29 insertions(+), 37 deletions(-) > >> > >> diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp > >> index fa266eca6..cbc85b59f 100644 > >> --- a/src/apps/cam/main.cpp > >> +++ b/src/apps/cam/main.cpp > >> @@ -13,6 +13,8 @@ > >> #include <signal.h> > >> #include <string.h> > >> > >> +#include <sys/signalfd.h> > > > > You can keep this grouped with the previous headers. > > > >> + > >> #include <libcamera/libcamera.h> > >> #include <libcamera/property_ids.h> > >> > >> @@ -27,15 +29,12 @@ using namespace libcamera; > >> class CamApp > >> { > >> public: > >> - CamApp(); > >> - > >> - static CamApp *instance(); > >> + CamApp(EventLoop &loop); > >> > >> int init(int argc, char **argv); > >> void cleanup(); > >> > >> int exec(); > >> - void quit(); > >> > >> private: > >> void cameraAdded(std::shared_ptr<Camera> cam); > >> @@ -46,26 +45,17 @@ private: > >> > >> static std::string cameraName(const Camera *camera); > >> > >> - static CamApp *app_; > >> OptionsParser::Options options_; > >> > >> std::unique_ptr<CameraManager> cm_; > >> > >> std::atomic_uint loopUsers_; > >> - EventLoop loop_; > >> + EventLoop *loop_ = nullptr; > >> }; > >> > >> -CamApp *CamApp::app_ = nullptr; > >> - > >> -CamApp::CamApp() > >> - : loopUsers_(0) > >> -{ > >> - CamApp::app_ = this; > >> -} > >> - > >> -CamApp *CamApp::instance() > >> +CamApp::CamApp(EventLoop &loop) > >> + : loopUsers_(0), loop_(&loop) > >> { > >> - return CamApp::app_; > >> } > >> > >> int CamApp::init(int argc, char **argv) > >> @@ -103,11 +93,6 @@ int CamApp::exec() > >> return ret; > >> } > >> > >> -void CamApp::quit() > >> -{ > >> - loop_.exit(); > >> -} > >> - > >> int CamApp::parseOptions(int argc, char *argv[]) > >> { > >> StreamKeyValueParser streamKeyValue; > >> @@ -205,7 +190,7 @@ void CamApp::cameraRemoved(std::shared_ptr<Camera> cam) > >> void CamApp::captureDone() > >> { > >> if (--loopUsers_ == 0) > >> - EventLoop::instance()->exit(0); > >> + loop_->exit(0); > >> } > >> > >> int CamApp::run() > >> @@ -290,7 +275,7 @@ int CamApp::run() > >> } > >> > >> if (loopUsers_) > >> - loop_.exec(); > >> + loop_->exec(); > >> > >> /* 6. Stop capture. */ > >> for (const auto &session : sessions) { > >> @@ -345,29 +330,36 @@ std::string CamApp::cameraName(const Camera *camera) > >> return name; > >> } > >> > >> -namespace { > >> - > >> -void signalHandler([[maybe_unused]] int signal) > >> +int main(int argc, char **argv) > >> { > >> - std::cout << "Exiting" << std::endl; > >> - CamApp::instance()->quit(); > >> -} > >> + auto sigfd = [] { > >> + sigset_t ss; > >> > >> -} /* namespace */ > >> + sigemptyset(&ss); > >> + sigaddset(&ss, SIGINT); > >> + sigprocmask(SIG_BLOCK, &ss, nullptr); > >> > >> -int main(int argc, char **argv) > >> -{ > >> - CamApp app; > >> + return UniqueFD(signalfd(-1, &ss, SFD_CLOEXEC | SFD_NONBLOCK)); > >> + }(); > > > > Why a lambda ? > > > >> + if (!sigfd.isValid()) > >> + return EXIT_FAILURE; > >> + > >> + EventLoop loop; > >> + CamApp app(loop); > >> int ret; > >> > >> + loop.addFdEvent(sigfd.get(), EventLoop::Read, [&] { > >> + signalfd_siginfo si; > >> + std::ignore = read(sigfd.get(), &si, sizeof(si)); > >> + > >> + std::cout << "Exiting" << std::endl; > >> + loop.exit(0); > >> + }); > > > > It could make things simpler to handle this in the CamApp class, you > > won't have to change where the EventLoop is instantiated. > > > >> + > >> ret = app.init(argc, argv); > >> if (ret) > >> return ret == -EINTR ? 0 : EXIT_FAILURE; > >> > >> - struct sigaction sa = {}; > >> - sa.sa_handler = &signalHandler; > >> - sigaction(SIGINT, &sa, nullptr); > >> - > >> if (app.exec()) > >> return EXIT_FAILURE; > >>
2025. 08. 16. 1:02 keltezéssel, Laurent Pinchart írta: > On Sat, Aug 16, 2025 at 12:28:49AM +0200, Barnabás Pőcze wrote: >> 2025. 08. 16. 0:21 keltezéssel, Laurent Pinchart írta: >>> On Fri, Aug 15, 2025 at 04:12:37PM +0200, Barnabás Pőcze wrote: >>>> Use a signalfd to detect `SIGINT` instead of registering a signal handler in >>>> order to remove the `CamApp` singleton. This is better than using a normal >>>> signal handler: a signal can arrive after the `CamApp` object has been destroyed, >>>> leading to a use-after-free. Modifying the `CamApp::app_` in the destructor is >>>> not a good alternative because that would not be async signal safe (unless it >>>> is made atomic). >>> >>> While this is true, it's also not really much of an issue in my opinion, >>> or at least a very minor one. Now that the patch exists lets get it >>> merged, but you don't have to hunt for these kind of problems in cam or >>> qcam :-) >> >> I actually did run into this on multiple occasions. :( > > Do you mean crashes because a signal arrived when the application was > closing ? How did that happen ? Yes. Spamming ctrl+c to stop `cam`. I usually test with asan, and with that it is easy to trigger this issue; eventually it became a bit too annoying, hence this change. > >>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >>>> --- >>>> src/apps/cam/main.cpp | 66 +++++++++++++++++++------------------------ >>>> 1 file changed, 29 insertions(+), 37 deletions(-) >>>> >>>> diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp >>>> index fa266eca6..cbc85b59f 100644 >>>> --- a/src/apps/cam/main.cpp >>>> +++ b/src/apps/cam/main.cpp >>>> @@ -13,6 +13,8 @@ >>>> #include <signal.h> >>>> #include <string.h> >>>> >>>> +#include <sys/signalfd.h> >>> >>> You can keep this grouped with the previous headers. >>> >>>> + >>>> #include <libcamera/libcamera.h> >>>> #include <libcamera/property_ids.h> >>>> >>>> @@ -27,15 +29,12 @@ using namespace libcamera; >>>> class CamApp >>>> { >>>> public: >>>> - CamApp(); >>>> - >>>> - static CamApp *instance(); >>>> + CamApp(EventLoop &loop); >>>> >>>> int init(int argc, char **argv); >>>> void cleanup(); >>>> >>>> int exec(); >>>> - void quit(); >>>> >>>> private: >>>> void cameraAdded(std::shared_ptr<Camera> cam); >>>> @@ -46,26 +45,17 @@ private: >>>> >>>> static std::string cameraName(const Camera *camera); >>>> >>>> - static CamApp *app_; >>>> OptionsParser::Options options_; >>>> >>>> std::unique_ptr<CameraManager> cm_; >>>> >>>> std::atomic_uint loopUsers_; >>>> - EventLoop loop_; >>>> + EventLoop *loop_ = nullptr; >>>> }; >>>> >>>> -CamApp *CamApp::app_ = nullptr; >>>> - >>>> -CamApp::CamApp() >>>> - : loopUsers_(0) >>>> -{ >>>> - CamApp::app_ = this; >>>> -} >>>> - >>>> -CamApp *CamApp::instance() >>>> +CamApp::CamApp(EventLoop &loop) >>>> + : loopUsers_(0), loop_(&loop) >>>> { >>>> - return CamApp::app_; >>>> } >>>> >>>> int CamApp::init(int argc, char **argv) >>>> @@ -103,11 +93,6 @@ int CamApp::exec() >>>> return ret; >>>> } >>>> >>>> -void CamApp::quit() >>>> -{ >>>> - loop_.exit(); >>>> -} >>>> - >>>> int CamApp::parseOptions(int argc, char *argv[]) >>>> { >>>> StreamKeyValueParser streamKeyValue; >>>> @@ -205,7 +190,7 @@ void CamApp::cameraRemoved(std::shared_ptr<Camera> cam) >>>> void CamApp::captureDone() >>>> { >>>> if (--loopUsers_ == 0) >>>> - EventLoop::instance()->exit(0); >>>> + loop_->exit(0); >>>> } >>>> >>>> int CamApp::run() >>>> @@ -290,7 +275,7 @@ int CamApp::run() >>>> } >>>> >>>> if (loopUsers_) >>>> - loop_.exec(); >>>> + loop_->exec(); >>>> >>>> /* 6. Stop capture. */ >>>> for (const auto &session : sessions) { >>>> @@ -345,29 +330,36 @@ std::string CamApp::cameraName(const Camera *camera) >>>> return name; >>>> } >>>> >>>> -namespace { >>>> - >>>> -void signalHandler([[maybe_unused]] int signal) >>>> +int main(int argc, char **argv) >>>> { >>>> - std::cout << "Exiting" << std::endl; >>>> - CamApp::instance()->quit(); >>>> -} >>>> + auto sigfd = [] { >>>> + sigset_t ss; >>>> >>>> -} /* namespace */ >>>> + sigemptyset(&ss); >>>> + sigaddset(&ss, SIGINT); >>>> + sigprocmask(SIG_BLOCK, &ss, nullptr); >>>> >>>> -int main(int argc, char **argv) >>>> -{ >>>> - CamApp app; >>>> + return UniqueFD(signalfd(-1, &ss, SFD_CLOEXEC | SFD_NONBLOCK)); >>>> + }(); >>> >>> Why a lambda ? >>> >>>> + if (!sigfd.isValid()) >>>> + return EXIT_FAILURE; >>>> + >>>> + EventLoop loop; >>>> + CamApp app(loop); >>>> int ret; >>>> >>>> + loop.addFdEvent(sigfd.get(), EventLoop::Read, [&] { >>>> + signalfd_siginfo si; >>>> + std::ignore = read(sigfd.get(), &si, sizeof(si)); >>>> + >>>> + std::cout << "Exiting" << std::endl; >>>> + loop.exit(0); >>>> + }); >>> >>> It could make things simpler to handle this in the CamApp class, you >>> won't have to change where the EventLoop is instantiated. >>> >>>> + >>>> ret = app.init(argc, argv); >>>> if (ret) >>>> return ret == -EINTR ? 0 : EXIT_FAILURE; >>>> >>>> - struct sigaction sa = {}; >>>> - sa.sa_handler = &signalHandler; >>>> - sigaction(SIGINT, &sa, nullptr); >>>> - >>>> if (app.exec()) >>>> return EXIT_FAILURE; >>>> >
diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp index fa266eca6..cbc85b59f 100644 --- a/src/apps/cam/main.cpp +++ b/src/apps/cam/main.cpp @@ -13,6 +13,8 @@ #include <signal.h> #include <string.h> +#include <sys/signalfd.h> + #include <libcamera/libcamera.h> #include <libcamera/property_ids.h> @@ -27,15 +29,12 @@ using namespace libcamera; class CamApp { public: - CamApp(); - - static CamApp *instance(); + CamApp(EventLoop &loop); int init(int argc, char **argv); void cleanup(); int exec(); - void quit(); private: void cameraAdded(std::shared_ptr<Camera> cam); @@ -46,26 +45,17 @@ private: static std::string cameraName(const Camera *camera); - static CamApp *app_; OptionsParser::Options options_; std::unique_ptr<CameraManager> cm_; std::atomic_uint loopUsers_; - EventLoop loop_; + EventLoop *loop_ = nullptr; }; -CamApp *CamApp::app_ = nullptr; - -CamApp::CamApp() - : loopUsers_(0) -{ - CamApp::app_ = this; -} - -CamApp *CamApp::instance() +CamApp::CamApp(EventLoop &loop) + : loopUsers_(0), loop_(&loop) { - return CamApp::app_; } int CamApp::init(int argc, char **argv) @@ -103,11 +93,6 @@ int CamApp::exec() return ret; } -void CamApp::quit() -{ - loop_.exit(); -} - int CamApp::parseOptions(int argc, char *argv[]) { StreamKeyValueParser streamKeyValue; @@ -205,7 +190,7 @@ void CamApp::cameraRemoved(std::shared_ptr<Camera> cam) void CamApp::captureDone() { if (--loopUsers_ == 0) - EventLoop::instance()->exit(0); + loop_->exit(0); } int CamApp::run() @@ -290,7 +275,7 @@ int CamApp::run() } if (loopUsers_) - loop_.exec(); + loop_->exec(); /* 6. Stop capture. */ for (const auto &session : sessions) { @@ -345,29 +330,36 @@ std::string CamApp::cameraName(const Camera *camera) return name; } -namespace { - -void signalHandler([[maybe_unused]] int signal) +int main(int argc, char **argv) { - std::cout << "Exiting" << std::endl; - CamApp::instance()->quit(); -} + auto sigfd = [] { + sigset_t ss; -} /* namespace */ + sigemptyset(&ss); + sigaddset(&ss, SIGINT); + sigprocmask(SIG_BLOCK, &ss, nullptr); -int main(int argc, char **argv) -{ - CamApp app; + return UniqueFD(signalfd(-1, &ss, SFD_CLOEXEC | SFD_NONBLOCK)); + }(); + if (!sigfd.isValid()) + return EXIT_FAILURE; + + EventLoop loop; + CamApp app(loop); int ret; + loop.addFdEvent(sigfd.get(), EventLoop::Read, [&] { + signalfd_siginfo si; + std::ignore = read(sigfd.get(), &si, sizeof(si)); + + std::cout << "Exiting" << std::endl; + loop.exit(0); + }); + ret = app.init(argc, argv); if (ret) return ret == -EINTR ? 0 : EXIT_FAILURE; - struct sigaction sa = {}; - sa.sa_handler = &signalHandler; - sigaction(SIGINT, &sa, nullptr); - if (app.exec()) return EXIT_FAILURE;
Use a signalfd to detect `SIGINT` instead of registering a signal handler in order to remove the `CamApp` singleton. This is better than using a normal signal handler: a signal can arrive after the `CamApp` object has been destroyed, leading to a use-after-free. Modifying the `CamApp::app_` in the destructor is not a good alternative because that would not be async signal safe (unless it is made atomic). Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- src/apps/cam/main.cpp | 66 +++++++++++++++++++------------------------ 1 file changed, 29 insertions(+), 37 deletions(-) -- 2.50.1