Karim Sabri / gammalib

Commit 1356ecc58fa84d6072c1f3a041b9cd6d905202ea

Authored by Jürgen Knödlseder
1 parent 99bf5fea

Make parameter file reading and writing thread safe (#3287)

Inline Side-by-side
Showing 4 changed files with 120 additions and 107 deletions
ChangeLog
  View file @1356ecc
1   -2020-07-22
  1 +2020-07-29
2 2  
3 3 * Bug fix version 1.7.1 released
4 4 ================================
5 5  
  6 + Make parameter file reading and writing thread safe (#3287)
6 7 Add quotes to sky region names (#3280)
7 8 Fix broken power law flux and eflux computation bug (#3279)
8 9  
... ...
1 1 New Features and Important Changes in GammaLib 1.7.1
2 2  
3   -22 July 2020
  3 +29 July 2020
4 4  
5 5  
6 6 1. Introduction
... ... @@ -43,7 +43,9 @@ None
43 43  
44 44 4. Application module
45 45 ---------------------
46   -None
  46 +The reading and writing of the parameter file in GApplicationPars::read()
  47 +and GApplicationPars::write() was put in an OpenMP critical zone to assure
  48 +that the reading and writing is thread safe (#3287).
47 49  
48 50  
49 51 5. Base module
... ...
doc/source/admin/release_history/1.7.1.rst
  View file @1356ecc
1 1 .. _1.7.1:
2 2  
3   -GammaLib 1.7.1 release (22 July 2020)
  3 +GammaLib 1.7.1 release (29 July 2020)
4 4 =====================================
5 5  
6 6 Overview
... ... @@ -10,6 +10,8 @@ GammaLib 1.7.1 is a bug fix release for GammaLib release 1.7.
10 10  
11 11 The following bugs were fixed:
12 12  
  13 +* [`3287 <https://cta-redmine.irap.omp.eu/issues/3287>`_] -
  14 + Make parameter file reading and writing thread safe
13 15 * [`3280 <https://cta-redmine.irap.omp.eu/issues/3280>`_] -
14 16 Add quotes to sky region names
15 17 * [`3279 <https://cta-redmine.irap.omp.eu/issues/3279>`_] -
... ...
src/app/GApplicationPars.cpp
  View file @1356ecc
... ... @@ -1268,62 +1268,66 @@ std::string GApplicationPars::outpath(const std::string&amp; filename) const
1268 1268 ***************************************************************************/
1269 1269 void GApplicationPars::read(const std::string& filename)
1270 1270 {
1271   - // Allocate line buffer
1272   - const int n = 1000;
1273   - char line[n];
1274   -
1275   - // Trying to get file lock
1276   - #if defined(G_LOCK_PARFILE)
1277   - struct flock lock;
1278   - lock.l_type = F_RDLCK; // Want a read lock
1279   - lock.l_whence = SEEK_SET; // Want beginning of file
1280   - lock.l_start = 0; // No offset, lock entire file ...
1281   - lock.l_len = 0; // ... to the end
1282   - lock.l_pid = getpid(); // Current process ID
1283   - int fd;
1284   - if ((fd = open(filename.c_str(), O_RDONLY)) == -1) {
1285   - throw GException::par_file_open_error(G_READ, filename,
1286   - "Could not open file for locking.");
1287   - }
1288   - #if defined(G_CHECK_LOCK_PARFILE)
1289   - if (fcntl(fd, F_SETLKW, &lock) == -1) { // F_SETLKW: wait until unlocked
1290   - throw GException::par_file_open_error(G_READ, filename,
1291   - "Could not get a lock on the file.");
1292   - }
1293   - #else
1294   - fcntl(fd, F_SETLKW, &lock);
1295   - #endif
1296   - #endif
  1271 + // Put in OpenMP critical zone
  1272 + #pragma omp critical(GApplicationsPars_read)
  1273 + {
  1274 + // Allocate line buffer
  1275 + const int n = 1000;
  1276 + char line[n];
  1277 +
  1278 + // Trying to get file lock
  1279 + #if defined(G_LOCK_PARFILE)
  1280 + struct flock lock;
  1281 + lock.l_type = F_RDLCK; // Want a read lock
  1282 + lock.l_whence = SEEK_SET; // Want beginning of file
  1283 + lock.l_start = 0; // No offset, lock entire file ...
  1284 + lock.l_len = 0; // ... to the end
  1285 + lock.l_pid = getpid(); // Current process ID
  1286 + int fd;
  1287 + if ((fd = open(filename.c_str(), O_RDONLY)) == -1) {
  1288 + throw GException::par_file_open_error(G_READ, filename,
  1289 + "Could not open file for locking.");
  1290 + }
  1291 + #if defined(G_CHECK_LOCK_PARFILE)
  1292 + if (fcntl(fd, F_SETLKW, &lock) == -1) { // F_SETLKW: wait until unlocked
  1293 + throw GException::par_file_open_error(G_READ, filename,
  1294 + "Could not get a lock on the file.");
  1295 + }
  1296 + #else
  1297 + fcntl(fd, F_SETLKW, &lock);
  1298 + #endif
  1299 + #endif
1297 1300  
1298   - // Open parameter file
1299   - FILE* fptr = fopen(filename.c_str(), "r");
1300   - if (fptr == NULL) {
1301   - throw GException::par_file_open_error(G_READ, filename);
1302   - }
  1301 + // Open parameter file
  1302 + FILE* fptr = fopen(filename.c_str(), "r");
  1303 + if (fptr == NULL) {
  1304 + throw GException::par_file_open_error(G_READ, filename);
  1305 + }
1303 1306  
1304   - // Read lines
1305   - while (fgets(line, n, fptr) != NULL) {
1306   - m_parfile.push_back(std::string(line));
1307   - }
  1307 + // Read lines
  1308 + while (fgets(line, n, fptr) != NULL) {
  1309 + m_parfile.push_back(std::string(line));
  1310 + }
1308 1311  
1309   - // Close file
1310   - fclose(fptr);
  1312 + // Close file
  1313 + fclose(fptr);
1311 1314  
1312   - // Unlock file
1313   - #if defined(G_LOCK_PARFILE)
1314   - if (fd != -1) {
1315   - lock.l_type = F_UNLCK;
1316   - #if defined(G_CHECK_LOCK_PARFILE)
1317   - if (fcntl(fd, F_SETLK, &lock) == -1) {
1318   - throw GException::par_file_open_error(G_READ, filename,
1319   - "Could not unlock the file.");
  1315 + // Unlock file
  1316 + #if defined(G_LOCK_PARFILE)
  1317 + if (fd != -1) {
  1318 + lock.l_type = F_UNLCK;
  1319 + #if defined(G_CHECK_LOCK_PARFILE)
  1320 + if (fcntl(fd, F_SETLK, &lock) == -1) {
  1321 + throw GException::par_file_open_error(G_READ, filename,
  1322 + "Could not unlock the file.");
  1323 + }
  1324 + #else
  1325 + fcntl(fd, F_SETLK, &lock);
  1326 + #endif
  1327 + close(fd);
1320 1328 }
1321   - #else
1322   - fcntl(fd, F_SETLK, &lock);
1323 1329 #endif
1324   - close(fd);
1325 1330 }
1326   - #endif
1327 1331  
1328 1332 // Return
1329 1333 return;
... ... @@ -1343,42 +1347,23 @@ void GApplicationPars::read(const std::string&amp; filename)
1343 1347 ***************************************************************************/
1344 1348 void GApplicationPars::write(const std::string& filename) const
1345 1349 {
1346   - // Trying to get file lock. We have to do this after opening the file
1347   - // using the fopen function, as the file may not exist, hence it needs
1348   - // to be created first.
1349   - #if defined(G_LOCK_PARFILE)
1350   - struct flock lock;
1351   - lock.l_type = F_WRLCK; // Want a write lock
1352   - lock.l_whence = SEEK_SET; // Want beginning of file
1353   - lock.l_start = 0; // No offset, lock entire file ...
1354   - lock.l_len = 0; // ... to the end
1355   - lock.l_pid = getpid(); // Current process ID
1356   - int fd;
1357   - if ((fd = open(filename.c_str(), O_WRONLY)) != -1) {
1358   - #if defined(G_CHECK_LOCK_PARFILE)
1359   - if (fcntl(fd, F_SETLKW, &lock) == -1) { // F_SETLKW: wait until unlocked
1360   - throw GException::par_file_open_error(G_WRITE, filename,
1361   - "Could not get a lock on the file.");
1362   - }
1363   - #else
1364   - fcntl(fd, F_SETLKW, &lock);
1365   - #endif
1366   - }
1367   - #endif
1368   -
1369   - // Open parameter file.
1370   - FILE* fptr = fopen(filename.c_str(), "w");
1371   - if (fptr == NULL) {
1372   - throw GException::par_file_open_error(G_WRITE, filename);
1373   - }
1374   -
1375   - // If file is not locked then lock it now.
1376   - #if defined(G_LOCK_PARFILE)
1377   - if (fd == -1) {
  1350 + // Put in OpenMP critical zone
  1351 + #pragma omp critical(GApplicationsPars_write)
  1352 + {
  1353 + // Trying to get file lock. We have to do this after opening the file
  1354 + // using the fopen function, as the file may not exist, hence it needs
  1355 + // to be created first.
  1356 + #if defined(G_LOCK_PARFILE)
  1357 + struct flock lock;
  1358 + lock.l_type = F_WRLCK; // Want a write lock
  1359 + lock.l_whence = SEEK_SET; // Want beginning of file
  1360 + lock.l_start = 0; // No offset, lock entire file ...
  1361 + lock.l_len = 0; // ... to the end
  1362 + lock.l_pid = getpid(); // Current process ID
  1363 + int fd;
1378 1364 if ((fd = open(filename.c_str(), O_WRONLY)) != -1) {
1379 1365 #if defined(G_CHECK_LOCK_PARFILE)
1380 1366 if (fcntl(fd, F_SETLKW, &lock) == -1) { // F_SETLKW: wait until unlocked
1381   - fclose(fptr);
1382 1367 throw GException::par_file_open_error(G_WRITE, filename,
1383 1368 "Could not get a lock on the file.");
1384 1369 }
... ... @@ -1386,33 +1371,56 @@ void GApplicationPars::write(const std::string&amp; filename) const
1386 1371 fcntl(fd, F_SETLKW, &lock);
1387 1372 #endif
1388 1373 }
1389   - }
1390   - #endif
  1374 + #endif
1391 1375  
1392   - // Write lines
1393   - for (int i = 0; i < m_parfile.size(); ++i) {
1394   - fprintf(fptr, "%s", m_parfile[i].c_str());
1395   - }
  1376 + // Open parameter file.
  1377 + FILE* fptr = fopen(filename.c_str(), "w");
  1378 + if (fptr == NULL) {
  1379 + throw GException::par_file_open_error(G_WRITE, filename);
  1380 + }
1396 1381  
1397   - // Close file
1398   - fclose(fptr);
  1382 + // If file is not locked then lock it now.
  1383 + #if defined(G_LOCK_PARFILE)
  1384 + if (fd == -1) {
  1385 + if ((fd = open(filename.c_str(), O_WRONLY)) != -1) {
  1386 + #if defined(G_CHECK_LOCK_PARFILE)
  1387 + if (fcntl(fd, F_SETLKW, &lock) == -1) { // F_SETLKW: wait until unlocked
  1388 + fclose(fptr);
  1389 + throw GException::par_file_open_error(G_WRITE, filename,
  1390 + "Could not get a lock on the file.");
  1391 + }
  1392 + #else
  1393 + fcntl(fd, F_SETLKW, &lock);
  1394 + #endif
  1395 + }
  1396 + }
  1397 + #endif
1399 1398  
1400   - // Unlock file
1401   - #if defined(G_LOCK_PARFILE)
1402   - if (fd != -1) {
1403   - lock.l_type = F_UNLCK;
1404   - #if defined(G_CHECK_LOCK_PARFILE)
1405   - if (fcntl(fd, F_SETLK, &lock) == -1) {
1406   - throw GException::par_file_open_error(G_WRITE, filename,
1407   - "Could not unlock the file.");
  1399 + // Write lines
  1400 + for (int i = 0; i < m_parfile.size(); ++i) {
  1401 + fprintf(fptr, "%s", m_parfile[i].c_str());
  1402 + }
  1403 +
  1404 + // Close file
  1405 + fclose(fptr);
  1406 +
  1407 + // Unlock file
  1408 + #if defined(G_LOCK_PARFILE)
  1409 + if (fd != -1) {
  1410 + lock.l_type = F_UNLCK;
  1411 + #if defined(G_CHECK_LOCK_PARFILE)
  1412 + if (fcntl(fd, F_SETLK, &lock) == -1) {
  1413 + throw GException::par_file_open_error(G_WRITE, filename,
  1414 + "Could not unlock the file.");
  1415 + }
  1416 + #else
  1417 + fcntl(fd, F_SETLK, &lock);
  1418 + #endif
  1419 + close(fd);
1408 1420 }
1409   - #else
1410   - fcntl(fd, F_SETLK, &lock);
1411 1421 #endif
1412   - close(fd);
1413 1422 }
1414   - #endif
1415   -
  1423 +
1416 1424 // Return
1417 1425 return;
1418 1426 }
... ...