Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Remove signal trapping by ViZDoom thread #599

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions docs/api/cpp/exceptions.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ The declarations of all the enums can be found in the `include/ViZDoomExceptions

* `MessageQueueException/SharedMemoryException` - means that communication with ViZDoom's instance failed. Usually, means a problem with permissions or system configuration.

* `SignalException` - means that a signal was cached by ViZDoom's instance.

* `ViZDoomErrorException` - means that an error in the ViZDoom engine occurred.

* `ViZDoomIsNotRunningException` - means that called method cannot be used when ViZDoom instance is not running.
Expand Down
6 changes: 0 additions & 6 deletions docs/api/python/exceptions.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,6 @@ Means that communication with ViZDoom's instance failed. Usually, means a proble

Means that allocation/reading of shared memory failed. Usually, means a problem with permissions or system configuration.

```{eval-rst}
.. autoexception:: vizdoom.SignalException
```

Means that a signal was cached by ViZDoom's instance.

```{eval-rst}
.. autoexception:: vizdoom.ViZDoomErrorException
```
Expand Down
10 changes: 1 addition & 9 deletions include/ViZDoomExceptions.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/*
Copyright (C) 2016 by Wojciech Jaśkowski, Michał Kempka, Grzegorz Runc, Jakub Toczek, Marek Wydmuch
Copyright (C) 2017 - 2022 by Marek Wydmuch, Michał Kempka, Wojciech Jaśkowski, and the respective contributors
Copyright (C) 2023 - 2024 by Marek Wydmuch, Farama Foundation, and the respective contributors

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -58,15 +59,6 @@ namespace vizdoom{
std::string error;
};

class SignalException : public std::exception {
public:
SignalException(std::string signal): signal(signal){}
~SignalException() throw(){}
const char* what() const throw();
private:
std::string signal;
};

class ViZDoomErrorException : public std::exception {
public:
ViZDoomErrorException(){}
Expand Down
57 changes: 1 addition & 56 deletions src/lib/ViZDoomController.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/*
Copyright (C) 2016 by Wojciech Jaśkowski, Michał Kempka, Grzegorz Runc, Jakub Toczek, Marek Wydmuch
Copyright (C) 2017 - 2022 by Marek Wydmuch, Michał Kempka, Wojciech Jaśkowski, and the respective contributors
Copyright (C) 2023 - 2024 by Marek Wydmuch, Farama Foundation, and the respective contributors

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -57,7 +58,6 @@ namespace vizdoom {
this->automapBuffer = nullptr;

/* Threads */
this->signalThread = nullptr;
this->doomThread = nullptr;

/* Flow control */
Expand Down Expand Up @@ -157,9 +157,6 @@ namespace vizdoom {
this->MQDoom = new MessageQueue(MQ_DOOM_NAME_BASE + this->instanceId);
this->MQController = new MessageQueue(MQ_CTR_NAME_BASE + this->instanceId);

// Signal handle thread
this->signalThread = new b::thread(b::bind(&DoomController::handleSignals, this));

// Doom thread
this->doomThread = new b::thread(b::bind(&DoomController::launchDoom, this));
this->doomRunning = true;
Expand Down Expand Up @@ -221,18 +218,6 @@ namespace vizdoom {
this->MQDoom->send(MSG_CODE_CLOSE);
}

if (this->signalThread && this->signalThread->joinable()) {
this->ioService->stop();

this->signalThread->interrupt();
this->signalThread->join();
delete this->signalThread;
this->signalThread = nullptr;

delete this->ioService;
this->ioService = nullptr;
}

#ifdef OS_POSIX
// If the Doom thread is still running, kill the engine process
if (this->doomThread && !this->doomThread->joinable()) {
Expand Down Expand Up @@ -1142,34 +1127,6 @@ namespace vizdoom {
}
}


/* Signals */
/*----------------------------------------------------------------------------------------------------------------*/

void DoomController::handleSignals() {
this->ioService = new ba::io_service();
ba::signal_set signals(*this->ioService, SIGINT, SIGABRT, SIGTERM);

#if BOOST_VERSION >= 106000
signals.async_wait(b::bind(signalHandler, b::ref(signals), this, bpl::_1, bpl::_2));
#else
signals.async_wait(b::bind(signalHandler, b::ref(signals), this, _1, _2));
#endif

this->ioService->run();
}

void DoomController::signalHandler(ba::signal_set &signal, DoomController *controller, const bs::error_code &error,
int sigNumber) {
controller->intSignal(sigNumber);
}

void DoomController::intSignal(int sigNumber) {
this->MQDoom->send(MSG_CODE_CLOSE);
this->MQController->send(static_cast<uint8_t >(MSG_CODE_SIG + sigNumber));
}


/* Flow */
/*----------------------------------------------------------------------------------------------------------------*/

Expand All @@ -1191,18 +1148,6 @@ namespace vizdoom {
this->close();
throw ViZDoomErrorException(std::string(msg.command));

case MSG_CODE_SIGINT :
this->close();
throw SignalException("SIGINT");

case MSG_CODE_SIGABRT :
this->close();
throw SignalException("SIGABRT");

case MSG_CODE_SIGTERM :
this->close();
throw SignalException("SIGTERM");

default:
this->close();
throw MessageQueueException("Unknown message code. Possible ViZDoom version mismatch.");
Expand Down
21 changes: 2 additions & 19 deletions src/lib/ViZDoomController.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
/*
Copyright (C) 2016 by Wojciech Jaśkowski, Michał Kempka, Grzegorz Runc, Jakub Toczek, Marek Wydmuch
Copyright (C) 2017 - 2022 by Marek Wydmuch, Michał Kempka, Wojciech Jaśkowski, and the respective contributors
Copyright (C) 2023 - 2024 by Marek Wydmuch, Farama Foundation, and the respective contributors


Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -75,24 +77,15 @@ namespace vizdoom {
#define MSG_CODE_COMMAND 24
#define MSG_CODE_CLOSE 25

#define MSG_CODE_SIG 30
#define MSG_CODE_SIGINT 30 + SIGINT
#define MSG_CODE_SIGABRT 30 + SIGABRT
#define MSG_CODE_SIGTERM 30 + SIGTERM

/* OSes */
#ifdef __linux__
#define OS_LINUX
#define OS_POSIX
#include <sys/types.h>
#include <signal.h>
#elif _WIN32
#define OS_WIN
#elif __APPLE__
#define OS_OSX
#define OS_POSIX
#include <sys/types.h>
#include <signal.h>
#endif

class DoomController {
Expand Down Expand Up @@ -322,16 +315,6 @@ namespace vizdoom {
/* Threads */
/*------------------------------------------------------------------------------------------------------------*/

ba::io_service *ioService;
b::thread *signalThread;

void handleSignals();

static void signalHandler(ba::signal_set &signal, DoomController *controller,
const bs::error_code &error, int sigNumber);

void intSignal(int sigNumber);

b::thread *doomThread;

#ifdef OS_POSIX
Expand Down
7 changes: 1 addition & 6 deletions src/lib/ViZDoomExceptions.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/*
Copyright (C) 2016 by Wojciech Jaśkowski, Michał Kempka, Grzegorz Runc, Jakub Toczek, Marek Wydmuch
Copyright (C) 2017 - 2022 by Marek Wydmuch, Michał Kempka, Wojciech Jaśkowski, and the respective contributors
Copyright (C) 2023 - 2024 by Marek Wydmuch, Farama Foundation, and the respective contributors

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -44,12 +45,6 @@ namespace vizdoom {
else return "Unknown shared memory error.";
}

/* SignalException */
const char *SignalException::what() const throw() {
std::string what = "Signal " + this->signal + " received. ViZDoom instance has been closed.";
return strdup(what.c_str());
}

/* ViZDoomErrorException */
const char *ViZDoomErrorException::what() const throw() {
if (this->error.length()) return this->error.c_str();
Expand Down
2 changes: 1 addition & 1 deletion src/lib_python/ViZDoomPythonModule.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/*
Copyright (C) 2016 by Wojciech Jaśkowski, Michał Kempka, Grzegorz Runc, Jakub Toczek, Marek Wydmuch
Copyright (C) 2017 - 2022 by Marek Wydmuch, Michał Kempka, Wojciech Jaśkowski, and the respective contributors
Copyright (C) 2023 - 2024 by Marek Wydmuch, Farama Foundation, and the respective contributors

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -59,7 +60,6 @@ PYBIND11_MODULE(vizdoom, vz){
EXCEPTION_TO_PYT(FileDoesNotExistException)
EXCEPTION_TO_PYT(MessageQueueException)
EXCEPTION_TO_PYT(SharedMemoryException)
EXCEPTION_TO_PYT(SignalException)
EXCEPTION_TO_PYT(ViZDoomIsNotRunningException)
EXCEPTION_TO_PYT(ViZDoomErrorException)
EXCEPTION_TO_PYT(ViZDoomUnexpectedExitException)
Expand Down
Loading