detectors/slsDetectorPackage
25
0
Fork 0
mirror of https://github.com/slsdetectorgroup/slsDetectorPackage.git synced 2025-04-20 02:40:03 +02:00
Code Issues Actions Packages Releases Activity

Refactoring of SharedMemory.h (#418)

Browse Source 
Function names match Detector.h
Removed double print due to LOG then throw
file descriptor not kept as a member variable
This commit is contained in:
Erik Fröjdh 2022-03-28 16:13:56 +02:00 committed by GitHub
parent 66900da476
commit e55e18d5e9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 149 additions and 227 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
slsDetectorSoftware/src/Detector.cpp
View File

@ -26,8 +26,8 @@ void freeSharedMemory(int detectorIndex, int moduleIndex) {
// single module
if (moduleIndex >= 0) {
SharedMemory<sharedModule> moduleShm(detectorIndex, moduleIndex);
if (moduleShm.IsExisting()) {
moduleShm.RemoveSharedMemory();
if (moduleShm.exists()) {
moduleShm.removeSharedMemory();
}
return;
}
@ -36,21 +36,21 @@ void freeSharedMemory(int detectorIndex, int moduleIndex) {
SharedMemory<sharedDetector> detectorShm(detectorIndex, -1);
int numDetectors = 0;
if (detectorShm.IsExisting()) {
detectorShm.OpenSharedMemory();
if (detectorShm.exists()) {
detectorShm.openSharedMemory();
numDetectors = detectorShm()->numberOfModules;
detectorShm.RemoveSharedMemory();
detectorShm.removeSharedMemory();
}
for (int i = 0; i < numDetectors; ++i) {
SharedMemory<sharedModule> moduleShm(detectorIndex, i);
moduleShm.RemoveSharedMemory();
moduleShm.removeSharedMemory();
}
// Ctb configuration
SharedMemory<CtbConfig> ctbShm(detectorIndex, -1, CtbConfig::shm_tag());
if (ctbShm.IsExisting())
ctbShm.RemoveSharedMemory();
if (ctbShm.exists())
ctbShm.removeSharedMemory();
}
using defs = slsDetectorDefs;

38
slsDetectorSoftware/src/DetectorImpl.cpp
View File

@ -46,8 +46,8 @@ void DetectorImpl::setupDetector(bool verify, bool update) {
updateUserdetails();
}
if (ctb_shm.IsExisting())
ctb_shm.OpenSharedMemory();
if (ctb_shm.exists())
ctb_shm.openSharedMemory();
}
void DetectorImpl::setAcquiringFlag(bool flag) { shm()->acquiringFlag = flag; }
@ -58,8 +58,8 @@ void DetectorImpl::freeSharedMemory(int detectorIndex, int detPos) {
// single
if (detPos >= 0) {
SharedMemory<sharedModule> moduleShm(detectorIndex, detPos);
if (moduleShm.IsExisting()) {
moduleShm.RemoveSharedMemory();
if (moduleShm.exists()) {
moduleShm.removeSharedMemory();
}
return;
}
@ -68,20 +68,20 @@ void DetectorImpl::freeSharedMemory(int detectorIndex, int detPos) {
SharedMemory<sharedDetector> detectorShm(detectorIndex, -1);
int numModules = 0;
if (detectorShm.IsExisting()) {
detectorShm.OpenSharedMemory();
if (detectorShm.exists()) {
detectorShm.openSharedMemory();
numModules = detectorShm()->numberOfModules;
detectorShm.RemoveSharedMemory();
detectorShm.removeSharedMemory();
}
for (int i = 0; i < numModules; ++i) {
SharedMemory<sharedModule> moduleShm(detectorIndex, i);
moduleShm.RemoveSharedMemory();
moduleShm.removeSharedMemory();
}
SharedMemory<CtbConfig> ctbShm(detectorIndex, -1, CtbConfig::shm_tag());
if (ctbShm.IsExisting())
ctbShm.RemoveSharedMemory();
if (ctbShm.exists())
ctbShm.removeSharedMemory();
}
void DetectorImpl::freeSharedMemory() {
@ -92,11 +92,11 @@ void DetectorImpl::freeSharedMemory() {
modules.clear();
// clear detector shm
shm.RemoveSharedMemory();
shm.removeSharedMemory();
client_downstream = false;
if (ctb_shm.IsExisting())
ctb_shm.RemoveSharedMemory();
if (ctb_shm.exists())
ctb_shm.removeSharedMemory();
}
std::string DetectorImpl::getUserDetails() {
@ -140,11 +140,11 @@ void DetectorImpl::setInitialChecks(const bool value) {
}
void DetectorImpl::initSharedMemory(bool verify) {
if (!shm.IsExisting()) {
shm.CreateSharedMemory();
if (!shm.exists()) {
shm.createSharedMemory();
initializeDetectorStructure();
} else {
shm.OpenSharedMemory();
shm.openSharedMemory();
if (verify && shm()->shmversion != DETECTOR_SHMVERSION) {
LOG(logERROR) << "Detector shared memory (" << detectorIndex
<< ") version mismatch "
@ -263,10 +263,10 @@ void DetectorImpl::setHostname(const std::vector<std::string> &name) {
// if needed, CTB dac names are only on detector level
if (shm()->detType == defs::CHIPTESTBOARD) {
if (ctb_shm.IsExisting())
ctb_shm.OpenSharedMemory();
if (ctb_shm.exists())
ctb_shm.openSharedMemory();
else
ctb_shm.CreateSharedMemory();
ctb_shm.createSharedMemory();
}
}

24
slsDetectorSoftware/src/Module.cpp
View File

@ -33,11 +33,11 @@ Module::Module(detectorType type, int det_id, int module_index, bool verify)
: moduleIndex(module_index), shm(det_id, module_index) {
// ensure shared memory was not created before
if (shm.IsExisting()) {
if (shm.exists()) {
LOG(logWARNING) << "This shared memory should have been "
"deleted before! "
<< shm.GetName() << ". Freeing it again";
shm.RemoveSharedMemory();
<< shm.getName() << ". Freeing it again";
shm.removeSharedMemory();
}
initSharedMemory(type, det_id, verify);
@ -55,8 +55,8 @@ Module::Module(int det_id, int module_index, bool verify)
Module::~Module() = default;
void Module::freeSharedMemory() {
if (shm.IsExisting()) {
shm.RemoveSharedMemory();
if (shm.exists()) {
shm.removeSharedMemory();
}
}
@ -3171,20 +3171,20 @@ Ret Module::sendToReceiver(int fnum, const Arg &args) {
slsDetectorDefs::detectorType Module::getDetectorTypeFromShm(int det_id,
bool verify) {
if (!shm.IsExisting()) {
throw SharedMemoryError("Shared memory " + shm.GetName() +
if (!shm.exists()) {
throw SharedMemoryError("Shared memory " + shm.getName() +
"does not exist.\n Corrupted Multi Shared "
"memory. Please free shared memory.");
}
shm.OpenSharedMemory();
shm.openSharedMemory();
if (verify && shm()->shmversion != MODULE_SHMVERSION) {
std::ostringstream ss;
ss << "Single shared memory (" << det_id << "-" << moduleIndex
<< ":)version mismatch (expected 0x" << std::hex << MODULE_SHMVERSION
<< " but got 0x" << shm()->shmversion << ")" << std::dec
<< ". Clear Shared memory to continue.";
shm.UnmapSharedMemory();
shm.unmapSharedMemory();
throw SharedMemoryError(ss.str());
}
return shm()->detType;
@ -3192,11 +3192,11 @@ slsDetectorDefs::detectorType Module::getDetectorTypeFromShm(int det_id,
void Module::initSharedMemory(detectorType type, int det_id, bool verify) {
shm = SharedMemory<sharedModule>(det_id, moduleIndex);
if (!shm.IsExisting()) {
shm.CreateSharedMemory();
if (!shm.exists()) {
shm.createSharedMemory();
initializeModuleStructure(type);
} else {
shm.OpenSharedMemory();
shm.openSharedMemory();
if (verify && shm()->shmversion != MODULE_SHMVERSION) {
std::ostringstream ss;
ss << "Single shared memory (" << det_id << "-" << moduleIndex

211
slsDetectorSoftware/src/SharedMemory.h
View File

@ -13,7 +13,8 @@
#include "sls/logger.h"
#include "sls/sls_detector_exceptions.h"
#include "stdlib.h"
// #include "stdlib.h"
#include <cstdlib>
#include <cerrno> // errno
#include <cstring> // strerror
#include <fcntl.h> // O_CREAT, O_TRUNC..
@ -33,268 +34,178 @@
namespace sls {
template <typename T> class SharedMemory {
static constexpr int NAME_MAX_LENGTH = 255;
std::string name;
T *shared_struct{};
public:
// moduleid of -1 creates a detector only shared memory
SharedMemory(int detectorId, int moduleIndex, const std::string &tag = "") {
name = ConstructSharedMemoryName(detectorId, moduleIndex, tag);
name = constructSharedMemoryName(detectorId, moduleIndex, tag);
}
/**
* Delete the copy constructor and copy assignment since we don't want two
* objects managing the same resource
*/
// Disable copy, since we refer to a unique location
SharedMemory(const SharedMemory &) = delete;
SharedMemory &operator=(const SharedMemory &other) = delete;
// Move constructor
SharedMemory(SharedMemory &&other)
: name(other.name), fd(other.fd), shmSize(other.shmSize),
shared_struct(other.shared_struct) {
other.fd = -1;
: name(other.name), shared_struct(other.shared_struct) {
other.shared_struct = nullptr;
other.shmSize = 0;
}
// Move assignment
SharedMemory &operator=(SharedMemory &&other) {
name = other.name;
if (fd) {
close(fd);
}
fd = other.fd;
other.fd = -1;
if (shared_struct != nullptr) {
UnmapSharedMemory();
}
if (shared_struct != nullptr)
unmapSharedMemory();
shared_struct = other.shared_struct;
other.shared_struct = nullptr;
shmSize = other.shmSize;
other.shmSize = 0;
return *this;
}
~SharedMemory() {
if (fd >= 0)
close(fd);
if (shared_struct) {
UnmapSharedMemory();
}
if (shared_struct)
unmapSharedMemory();
}
/**
* Verify if it exists
* @return true if exists, else false
*/
bool IsExisting() {
bool ret = true;
T *operator()() { return shared_struct; }
const T *operator()() const { return shared_struct; }
std::string getName() const { return name; }
bool exists() {
int tempfd = shm_open(name.c_str(), O_RDWR, 0);
if ((tempfd < 0) && (errno == ENOENT)) {
ret = false;
return false;
}
close(tempfd);
return ret;
return true;
}
std::string GetName() const { return name; }
size_t size() const { return shmSize; }
/**
* Create Shared memory and call MapSharedMemory to map it to an address
* throws a SharedMemoryError exception on failure to create, ftruncate or
* map
*/
void CreateSharedMemory() {
fd = shm_open(name.c_str(), O_CREAT | O_TRUNC | O_EXCL | O_RDWR,
void createSharedMemory() {
int fd = shm_open(name.c_str(), O_CREAT | O_TRUNC | O_EXCL | O_RDWR,
S_IRUSR | S_IWUSR);
if (fd < 0) {
std::string msg =
"Create shared memory " + name + " failed: " + strerror(errno);
LOG(logERROR) << msg;
throw SharedMemoryError(msg);
}
if (ftruncate(fd, sizeof(T)) < 0) {
std::string msg = "Create shared memory " + name +
" failed at ftruncate: " + strerror(errno);
LOG(logERROR) << msg;
close(fd);
RemoveSharedMemory();
removeSharedMemory();
throw SharedMemoryError(msg);
}
shared_struct = MapSharedMemory();
shared_struct = mapSharedMemory(fd);
new (shared_struct) T{};
LOG(logINFO) << "Shared memory created " << name;
}
/**
* Open existing Shared memory and call MapSharedMemory to map it to an
* address throws a SharedMemoryError exception on failure to open or map
*/
void OpenSharedMemory() {
fd = shm_open(name.c_str(), O_RDWR, 0);
void openSharedMemory() {
int fd = shm_open(name.c_str(), O_RDWR, 0);
if (fd < 0) {
std::string msg = "Open existing shared memory " + name +
" failed: " + strerror(errno);
LOG(logERROR) << msg;
throw SharedMemoryError(msg);
}
shared_struct = MapSharedMemory();
checkSize(fd);
shared_struct = mapSharedMemory(fd);
}
/**
* Unmap shared memory from an address
* throws a SharedMemoryError exception on failure
*/
void UnmapSharedMemory() {
void unmapSharedMemory() {
if (shared_struct != nullptr) {
if (munmap(shared_struct, shmSize) < 0) {
if (munmap(shared_struct, sizeof(T)) < 0) {
std::string msg = "Unmapping shared memory " + name +
" failed: " + strerror(errno);
LOG(logERROR) << msg;
close(fd);
throw SharedMemoryError(msg);
}
shared_struct = nullptr;
}
}
/**
* Remove existing Shared memory
*/
void RemoveSharedMemory() {
UnmapSharedMemory();
void removeSharedMemory() {
unmapSharedMemory();
if (shm_unlink(name.c_str()) < 0) {
// silent exit if shm did not exist anyway
if (errno == ENOENT)
return;
std::string msg =
"Free Shared Memory " + name + " Failed: " + strerror(errno);
LOG(logERROR) << msg;
throw SharedMemoryError(msg);
}
LOG(logINFO) << "Shared memory deleted " << name;
}
/**
* Maximum length of name as from man pages
*/
static const int NAME_MAX_LENGTH = 255;
/**
*Using the call operator to access the pointer
*/
T *operator()() { return shared_struct; }
/**
*Using the call operator to access the pointer, const overload
*/
const T *operator()() const { return shared_struct; }
private:
/**
* Create Shared memory name
* throws exception if name created is longer than required 255(manpages)
* @param detectorId detector id
* @param moduleIndex module id, -1 if a detector shared memory
* @returns shared memory name
*/
std::string ConstructSharedMemoryName(int detectorId, int moduleIndex, const std::string& tag) {
std::string constructSharedMemoryName(int detectorId, int moduleIndex,
const std::string &tag) {
// using environment path
std::string sEnvPath;
// using environment variable
std::string slsdetname;
char *envpath = getenv(SHM_ENV_NAME);
if (envpath != nullptr) {
sEnvPath.assign(envpath);
sEnvPath.insert(0, "_");
slsdetname = envpath;
slsdetname.insert(0, "_");
}
std::stringstream ss;
if (moduleIndex < 0) {
ss << SHM_DETECTOR_PREFIX << detectorId << sEnvPath;
ss << SHM_DETECTOR_PREFIX << detectorId << slsdetname;
if (!tag.empty())
ss << "_" << tag;
} else {
ss << SHM_DETECTOR_PREFIX << detectorId << SHM_MODULE_PREFIX
<< moduleIndex << slsdetname;
}
else
ss << SHM_DETECTOR_PREFIX << detectorId << SHM_MODULE_PREFIX
<< moduleIndex << sEnvPath;
std::string temp = ss.str();
if (temp.length() > NAME_MAX_LENGTH) {
std::string shm_name = ss.str();
if (shm_name.length() > NAME_MAX_LENGTH) {
std::string msg =
"Shared memory initialization failed. " + temp + " has " +
std::to_string(temp.length()) + " characters. \n" +
"Shared memory initialization failed. " + shm_name + " has " +
std::to_string(shm_name.length()) + " characters. \n" +
"Maximum is " + std::to_string(NAME_MAX_LENGTH) +
". Change the environment variable " + SHM_ENV_NAME;
LOG(logERROR) << msg;
throw SharedMemoryError(msg);
}
return temp;
return shm_name;
}
/**
* Map shared memory to an address
* throws a SharedMemoryException exception on failure
*/
T *MapSharedMemory() {
// from the Linux manual:
// After the mmap() call has returned, the file descriptor, fd, can
// be closed immediately without invalidating the mapping.
T *mapSharedMemory(int fd) {
void *addr =
mmap(nullptr, sizeof(T), PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
close(fd);
if (addr == MAP_FAILED) {
std::string msg =
"Mapping shared memory " + name + " failed: " + strerror(errno);
LOG(logERROR) << msg;
close(fd);
throw SharedMemoryError(msg);
}
shmSize = sizeof(T);
close(fd);
return (T *)addr;
return static_cast<T *>(addr);
}
/**
* Verify if existing shared memory size matches expected size
* @param expectedSize expected size of shared memory, replaced with smaller
* size if size does not match
* @return 0 for success, 1 for fail
*/
int VerifySizeMatch(size_t expectedSize) {
void checkSize(int fd) {
struct stat sb;
// could not fstat
if (fstat(fd, &sb) < 0) {
std::string msg = "Could not verify existing shared memory " +
name + " size match " +
"(could not fstat): " + strerror(errno);
LOG(logERROR) << msg;
close(fd);
throw SharedMemoryError(msg);
}
// size does not match
auto sz = static_cast<size_t>(sb.st_size);
if (sz != expectedSize) {
std::string msg = "Existing shared memory " + name +
" size does not match" + "Expected " +
std::to_string(expectedSize) + ", found " +
std::to_string(sz);
LOG(logERROR) << msg;
auto actual_size = static_cast<size_t>(sb.st_size);
auto expected_size = sizeof(T);
if (actual_size != expected_size) {
std::string msg =
"Existing shared memory " + name + " size does not match. " +
"Expected " + std::to_string(expected_size) + ", found " +
std::to_string(actual_size) +
". Detector software mismatch? Try freeing shared memory.";
throw SharedMemoryError(msg);
return 1;
}
return 0;
}
std::string name;
int fd{-1};
size_t shmSize{0};
T *shared_struct{nullptr};
};
} // namespace sls

4
slsDetectorSoftware/tests/test-Module.cpp
View File

@ -32,8 +32,8 @@ TEST_CASE("Is shm fixed pattern shm compatible") {
// Set shm version to 0
sls::SharedMemory<sls::sharedModule> shm(0, 0);
REQUIRE(shm.IsExisting() == true);
shm.OpenSharedMemory();
REQUIRE(shm.exists() == true);
shm.openSharedMemory();
shm()->shmversion = 0;
// Should fail since version is set to 0

73
slsDetectorSoftware/tests/test-SharedMemory.cpp
View File

@ -20,8 +20,8 @@ constexpr int shm_id = 10;
TEST_CASE("Create SharedMemory read and write", "[detector]") {
SharedMemory<Data> shm(shm_id, -1);
shm.CreateSharedMemory();
CHECK(shm.GetName() == std::string("/slsDetectorPackage_detector_") +
shm.createSharedMemory();
CHECK(shm.getName() == std::string("/slsDetectorPackage_detector_") +
std::to_string(shm_id));
shm()->x = 3;
@ -32,25 +32,25 @@ TEST_CASE("Create SharedMemory read and write", "[detector]") {
CHECK(shm()->y == 5.7);
CHECK(std::string(shm()->mess) == "Some string");
shm.UnmapSharedMemory();
shm.RemoveSharedMemory();
shm.unmapSharedMemory();
shm.removeSharedMemory();
CHECK(shm.IsExisting() == false);
CHECK(shm.exists() == false);
}
TEST_CASE("Open existing SharedMemory and read", "[detector]") {
{
SharedMemory<double> shm(shm_id, -1);
shm.CreateSharedMemory();
shm.createSharedMemory();
*shm() = 5.3;
}
SharedMemory<double> shm2(shm_id, -1);
shm2.OpenSharedMemory();
shm2.openSharedMemory();
CHECK(*shm2() == 5.3);
shm2.RemoveSharedMemory();
shm2.removeSharedMemory();
}
TEST_CASE("Creating a second shared memory with the same name throws",
@ -59,24 +59,24 @@ TEST_CASE("Creating a second shared memory with the same name throws",
SharedMemory<double> shm0(shm_id, -1);
SharedMemory<double> shm1(shm_id, -1);
shm0.CreateSharedMemory();
CHECK_THROWS(shm1.CreateSharedMemory());
shm0.RemoveSharedMemory();
shm0.createSharedMemory();
CHECK_THROWS(shm1.createSharedMemory());
shm0.removeSharedMemory();
}
TEST_CASE("Open two shared memories to the same place", "[detector]") {
// Create the first shared memory
SharedMemory<Data> shm(shm_id, -1);
shm.CreateSharedMemory();
shm.createSharedMemory();
shm()->x = 5;
CHECK(shm()->x == 5);
// Open the second shared memory with the same name
SharedMemory<Data> shm2(shm_id, -1);
shm2.OpenSharedMemory();
shm2.openSharedMemory();
CHECK(shm2()->x == 5);
CHECK(shm.GetName() == shm2.GetName());
CHECK(shm.getName() == shm2.getName());
// Check that they still point to the same place
shm2()->x = 7;
@ -84,31 +84,28 @@ TEST_CASE("Open two shared memories to the same place", "[detector]") {
// Remove only needs to be done once since they refer
// to the same memory
shm2.RemoveSharedMemory();
CHECK(shm.IsExisting() == false);
CHECK(shm2.IsExisting() == false);
shm2.removeSharedMemory();
CHECK(shm.exists() == false);
CHECK(shm2.exists() == false);
}
TEST_CASE("Move SharedMemory", "[detector]") {
SharedMemory<Data> shm(shm_id, -1);
CHECK(shm.GetName() == std::string("/slsDetectorPackage_detector_") +
CHECK(shm.getName() == std::string("/slsDetectorPackage_detector_") +
std::to_string(shm_id));
shm.CreateSharedMemory();
shm.createSharedMemory();
shm()->x = 9;
CHECK(shm.size() == sizeof(Data));
SharedMemory<Data> shm2(shm_id + 1, -1);
shm2 = std::move(shm); // shm is now a moved from object!
CHECK(shm2()->x == 9);
CHECK(shm() == nullptr);
CHECK(shm.size() == 0);
CHECK(shm2.GetName() == std::string("/slsDetectorPackage_detector_") +
CHECK(shm2.getName() == std::string("/slsDetectorPackage_detector_") +
std::to_string(shm_id));
shm2.RemoveSharedMemory();
shm2.removeSharedMemory();
}
TEST_CASE("Create several shared memories", "[detector]") {
@ -117,27 +114,27 @@ TEST_CASE("Create several shared memories", "[detector]") {
v.reserve(N);
for (int i = 0; i != N; ++i) {
v.emplace_back(shm_id + i, -1);
CHECK(v[i].IsExisting() == false);
v[i].CreateSharedMemory();
CHECK(v[i].exists() == false);
v[i].createSharedMemory();
*v[i]() = i;
CHECK(*v[i]() == i);
}
for (int i = 0; i != N; ++i) {
CHECK(*v[i]() == i);
CHECK(v[i].GetName() == std::string("/slsDetectorPackage_detector_") +
CHECK(v[i].getName() == std::string("/slsDetectorPackage_detector_") +
std::to_string(i + shm_id));
}
for (int i = 0; i != N; ++i) {
v[i].RemoveSharedMemory();
CHECK(v[i].IsExisting() == false);
v[i].removeSharedMemory();
CHECK(v[i].exists() == false);
}
}
TEST_CASE("Create create a shared memory with a tag"){
SharedMemory<int> shm(0, -1, "ctbdacs");
REQUIRE(shm.GetName() == "/slsDetectorPackage_detector_0_ctbdacs");
REQUIRE(shm.getName() == "/slsDetectorPackage_detector_0_ctbdacs");
}
@ -152,7 +149,7 @@ TEST_CASE("Create create a shared memory with a tag when SLSDETNAME is set"){
setenv(SHM_ENV_NAME, "myprefix", 1);
SharedMemory<int> shm(0, -1, "ctbdacs");
REQUIRE(shm.GetName() == "/slsDetectorPackage_detector_0_myprefix_ctbdacs");
REQUIRE(shm.getName() == "/slsDetectorPackage_detector_0_myprefix_ctbdacs");
// Clean up after us
if (old_slsdetname.empty())
@ -162,3 +159,17 @@ TEST_CASE("Create create a shared memory with a tag when SLSDETNAME is set"){
}
TEST_CASE("map int64 to int32 throws"){
SharedMemory<int32_t> shm(shm_id, -1);
shm.createSharedMemory();
*shm() = 7;
SharedMemory<int64_t> shm2(shm_id, -1);
REQUIRE_THROWS(shm2.openSharedMemory());
shm.removeSharedMemory();
}