Skip to content

Commit 4ed13be

Browse files
meynardcmeynardc
committed
Fix multiprocess writes in same image (was failing with some gdal version)
1 parent aa7b29d commit 4ed13be

File tree

6 files changed

+261
-226
lines changed

6 files changed

+261
-226
lines changed

‎MMVII/include/FileLock.h‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
#ifndef FILELOCK_H
2+
#define FILELOCK_H
3+
4+
#include <string>
5+
6+
namespace MMVII {
7+
8+
/// @brief RAII file lock providing mutual exclusion between multiple processes
9+
/// accessing the same file.
10+
///
11+
/// On Linux, FileLock creates a companion lock file in a dedicated directory:
12+
/// cMMVII_Appli::CurrentAppli().DirProject() + MMVII::TmpMMVIIDirGlob + "/Locks/"
13+
/// and acquires a POSIX advisory write lock (fcntl F_SETLKW) on it.
14+
/// The lock directory is created on first use for each unique path.
15+
///
16+
/// On Windows, FileLock uses a named Mutex derived from the canonical path
17+
/// of the file to protect. If the path exceeds MAX_PATH, a hash-based name
18+
/// is used instead.
19+
///
20+
/// @note Locks are advisory: all cooperating processes must use FileLock consistently.
21+
/// @note On Linux, fcntl locks are per (pid, inode): threads within the same process
22+
/// do NOT block each other. Add a std::mutex if intra-process exclusion is needed.
23+
/// @note The lock is automatically released if the process crashes (kernel/OS cleanup).
24+
/// @note FileLock is non-copyable and non-movable.
25+
///
26+
/// @par Example
27+
/// @code
28+
/// {
29+
/// FileLock lock("data.txt");
30+
/// black_box_write("data.txt");
31+
/// } // lock released here
32+
/// @endcode
33+
class FileLock {
34+
public:
35+
/// @brief Acquires an exclusive lock on the given file.
36+
///
37+
/// On Linux, opens (or creates) a companion lock file and blocks until
38+
/// the POSIX write lock is obtained. The lock files are created in
39+
/// {DirProject}/MMVII-Tmp-Dir-Glob/Locks/
40+
///
41+
/// On Windows, creates or opens a named Mutex derived from the file's
42+
/// canonical path and waits indefinitely until it is acquired.
43+
///
44+
/// @param filepath Path to the file to protect. The file does not need
45+
/// to exist at the time the lock is acquired.
46+
/// @throws MMVII_INTERNAL_ERROR if the lock file cannot be opened
47+
/// or the lock cannot be acquired.
48+
explicit FileLock(const std::string& filepath);
49+
50+
/// @brief Releases the lock.
51+
///
52+
/// On Linux, unlocks the POSIX lock and closes the lock file descriptor.
53+
/// The companion lock file itself is NOT deleted on destruction.
54+
/// On Windows, releases and closes the Mutex handle.
55+
~FileLock();
56+
57+
FileLock(const FileLock&) = delete;
58+
FileLock& operator=(const FileLock&) = delete;
59+
60+
private:
61+
#ifdef WIN32
62+
void* mHandle = nullptr; ///< Windows Mutex handle.
63+
#else
64+
int lock_fd_; ///< File descriptor of the companion lock file (Linux).
65+
#endif
66+
};
67+
68+
} // namespace MMVII
69+
70+
#endif // FILELOCK_H

‎MMVII/include/MMVII_Sys.h‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -58,29 +58,6 @@ int GlobParalSysCallByMkF(const std::string & aNameMkF,const std::list<cParamCal
5858
std::string MMVII_CanonicalRootDirFromExec();
5959

6060

61-
class FileLock
62-
{
63-
public:
64-
FileLock();
65-
explicit FileLock(const std::string& name);
66-
~FileLock();
67-
bool lock(const std::string& name);
68-
void unlock();
69-
bool hasLock();
70-
71-
private:
72-
#ifdef WIN32
73-
void *mHandle = nullptr;
74-
#else
75-
int mFd = -1;
76-
#endif
77-
void doLock(const std::string& name);
78-
void doUnlock();
79-
bool doHasLock();
80-
};
81-
82-
83-
8461
};
8562

8663
#endif // _MMVII_Sys_H_

‎MMVII/src/Bench/BenchAPBI.cpp‎

Lines changed: 4 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ cAppliTestAPBI::cAppliTestAPBI
4545
{
4646
}
4747

48+
4849
cCollecSpecArg2007 & cAppliTestAPBI::ArgObl(cCollecSpecArg2007 & anArgObl)
4950
{
5051
return anArgObl;
@@ -65,14 +66,8 @@ int cAppliTestAPBI::ExeOnParsedBox()
6566
for (const auto & aPix : aImTest.DIm())
6667
{
6768
aImTest.DIm().SetVTrunc(aPix,(aVal + (int)imSrc.GetV(aPix)) % 255);
68-
69-
// aImTest.DIm().SetVTrunc(aPix,(aVal + (int)(aPix.x()+aPix.y())) % 255);
70-
7169
}
72-
//StdOut() << "LLLokcIn \n";
7370
APBI_WriteIm(ImNameOut(),aImTest);
74-
//StdOut() << "LLLokcOut \n\n";
75-
7671
return EXIT_SUCCESS;
7772
}
7873

@@ -114,20 +109,15 @@ void BenchAPBI(cParamExeBench & aParam)
114109
if (! aParam.NewBench("APBI")) return;
115110

116111

117-
/* cDataFileIm2D::Create(cAppliTestAPBI::ImNameIn(),eTyNums::eTN_U_INT1,
118-
cPt2di(2000+RandUnif_M_N(-100,100),2000+RandUnif_M_N(-100,100)),
119-
1
120-
);*/
121-
cPt2di aSz (2000+RandUnif_M_N(-100,100),2000+RandUnif_M_N(-100,100));
122-
auto aIm2d = cIm2D<tU_INT1>(aSz);
123-
cIm2D<tINT4> aImCpt(aSz,nullptr,eModeInitImage::eMIA_Null);
112+
cPt2di aSz (2000+RandUnif_M_N(-100,100),2000+RandUnif_M_N(-100,100));
113+
auto aBoxSize = cPt2di(200+RandUnif_M_N(-10,10),200+RandUnif_M_N(-10,10));
114+
auto aIm2d = cIm2D<tU_INT1>(aSz);
124115

125116
for (const auto& aPix : aIm2d.DIm()) {
126117
aIm2d.DIm().SetV(aPix,(aPix.x()+aPix.y()) % 255);
127118
}
128119
cDataFileIm2D aDF = cDataFileIm2D::Create(cAppliTestAPBI::ImNameIn(),eTyNums::eTN_U_INT1,aIm2d.DIm().Sz(),1);
129120
aIm2d.Write(aDF,cPt2di(0,0));
130-
auto aBoxSize = cPt2di(200+RandUnif_M_N(-10,10),200+RandUnif_M_N(-10,10));
131121

132122
cMMVII_Appli & anAp = cMMVII_Appli::CurrentAppli();
133123

@@ -138,35 +128,15 @@ void BenchAPBI(cParamExeBench & aParam)
138128
anAp.StrOpt() << std::make_pair("SzTiles",cStrIO<cPt2di>::ToStr(aBoxSize))
139129
);
140130

141-
int aNbDif = 0;
142-
int aNbInd = 0;
143-
144131
auto mIm2d = cIm2D<tU_INT1>::FromFile(cAppliTestAPBI::ImNameOut());
145132
cParseBoxInOut<2> aPBIO = cParseBoxInOut<2>::CreateFromSize(mIm2d.DIm(),aBoxSize);
146133
for (const auto & aPixI : aPBIO.BoxIndex()) {
147-
aNbInd++;
148134
auto aBox = aPBIO.BoxOut(aPixI);
149135
tU_INT1 aVal = cAppliTestAPBI::valFromBox(aBox);
150136
for (const auto &aPix : aBox) {
151-
/*
152-
StdOut() << "APBI=" << (int) mIm2d.DIm().GetV(aPix) << " " << (int) (aVal + (aPix.x() + aPix.y()) % 255)%255 << aPix << " " << aBoxSize << "\n";
153137
MMVII_INTERNAL_ASSERT_bench(mIm2d.DIm().GetV(aPix) == (aVal + (aPix.x() + aPix.y()) % 255)%255,"BenchAPBI failed");
154-
*/
155-
if (mIm2d.DIm().GetV(aPix) != (aVal + (aPix.x() + aPix.y()) % 255)%255)
156-
{
157-
/* StdOut() << "APBI=" << (int) mIm2d.DIm().GetV(aPix) << " "
158-
<< (int) (aVal + (aPix.x() + aPix.y()) % 255)%255
159-
<< aPix << " " << aBox.Sz() << "\n";*/
160-
aNbDif++;
161-
}
162-
aImCpt.DIm().AddVal(aPix,1);
163138
}
164139
}
165-
for (const auto aPix : mIm2d.DIm() )
166-
{
167-
MMVII_INTERNAL_ASSERT_bench(aImCpt.DIm().GetV(aPix)==1,"Non partion in ParseBox");
168-
}
169-
StdOut() << "GGettttChar " << cAppliTestAPBI::ImNameIn() << " NBDif=" << aNbDif << " NBI=" << aNbInd << "\n" ; //getchar();
170140
aParam.EndBench();
171141
}
172142

‎MMVII/src/ImagesBase/cGdalApi.h‎

Lines changed: 74 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88
#include "MMVII_Image2D.h"
99

1010
#include <string>
11-
#include <gdal_priv.h>
11+
#include "gdal_priv.h"
12+
#include "FileLock.h"
1213

1314
/*
1415
* GDAL API interface to MMVII to read/write image file
@@ -118,90 +119,99 @@ class GdalIO {
118119
, mGdalNbChan(0)
119120
, mNbImg(0)
120121
{}
121-
void operator()(IoMode aMode, const cDataFileIm2D &aDF, const tvIm& aVecImV2, const cPt2di & aP0File,double aDyn,const cRect2& aR2)
122+
void operator()(IoMode aMode, const cDataFileIm2D &aDF, const tvIm& aVecImV2, const cPt2di & aP0File, double aDyn, const cRect2& aR2)
122123
{
123-
mDataFile = &aDF;
124-
auto aName = mDataFile->Name();
125-
mGdalNbChan = mDataFile->NbChannel();
126-
mNbImg = static_cast<int>(aVecImV2.size());
124+
mDataFile = &aDF;
125+
mGdalNbChan = mDataFile->NbChannel();
126+
mNbImg = static_cast<int>(aVecImV2.size());
127127

128128
auto aIm2D = aVecImV2[0];
129129
cRect2 aRectFullIm(aIm2D->P0(), aIm2D->P1());
130-
cRect2 aRectIm = (aR2 == cRect2::TheEmptyBox) ? aRectFullIm : aR2;
131-
cRect2 aRectFile (aRectIm.Translate(aP0File)) ;
130+
cRect2 aRectIm = (aR2 == cRect2::TheEmptyBox) ? aRectFullIm : aR2;
131+
cRect2 aRectFile(aRectIm.Translate(aP0File));
132+
133+
auto aName = mDataFile->Name();
134+
MMVII_INTERNAL_ASSERT_strong(aRectFile.IncludedIn(aDF), "Read/write out of image file (" + aName + ")");
135+
MMVII_INTERNAL_ASSERT_strong(aRectIm.IncludedIn(aRectFullIm), "Read/write out of Image buffer (" + aName + ")");
136+
137+
if (aMode == IoMode::Read)
138+
{
139+
Read(aVecImV2, aRectIm, aRectFile, aDyn);
140+
}
141+
else
142+
{
143+
FileLock gdalLock(aName);
144+
Write(aVecImV2, aRectIm, aRectFile, aDyn);
145+
}
146+
}
132147

133-
MMVII_INTERNAL_ASSERT_strong(aRectFile.IncludedIn(aDF), "Read/write out of image file (" + aName + ")");
134-
MMVII_INTERNAL_ASSERT_strong(aRectIm.IncludedIn(aRectFullIm), "Read/write out of Image buffer (" + aName + ")");
148+
private:
149+
void Read(const tvIm& aVecImV2, const cRect2& aRectIm, const cRect2& aRectFile, double aDyn)
150+
{
151+
auto aName = mDataFile->Name();
152+
153+
if (mDataFile->IsCreateAtFirstWrite())
154+
MMVII_INTERNAL_ERROR("GDAL read: image file created with CreateOnWrite() must be written before trying to read it (" + aName + ")");
155+
156+
mGdalDataset = cGdalApi::OpenDataset(aName, GA_ReadOnly, cGdalApi::eOnError::RaiseError);
157+
158+
if (mGdalNbChan == mNbImg && mNbImg != 0)
159+
GdalReadNtoN(aVecImV2, aRectIm, aRectFile, aDyn); // file N -> N img channels
160+
else if (mGdalNbChan == 1 && mNbImg != 0)
161+
GdalRead1toN(aVecImV2, aRectIm, aRectFile, aDyn); // file 1 -> N img channels
162+
else if (mGdalNbChan != 0 && mNbImg == 1)
163+
GdalReadNto1(aVecImV2, aRectIm, aRectFile, aDyn); // file N -> 1 img channel
164+
else
165+
MMVII_INTERNAL_ERROR("Gdal read: Images vector size: " + std::to_string(mNbImg) + ", file channels: " + std::to_string(mGdalNbChan) + " (" + aName + ")");
166+
167+
cGdalApi::CloseDataset(mGdalDataset);
168+
}
169+
170+
void Write(const tvIm& aVecImV2, const cRect2& aRectIm, const cRect2& aRectFile, double aDyn)
171+
{
172+
auto aName = mDataFile->Name();
173+
bool notUpdatable = false;
135174

136-
auto notUpdatable = false;
137175
if (mDataFile->IsCreateAtFirstWrite())
138176
{
139-
if (aMode != IoMode::Write)
140-
{
141-
MMVII_INTERNAL_ERROR("GDAL read: image file created with CreateOnWrite() must be written before trying to read it (" + aName + ")");
142-
}
143-
if (aRectFile.Sz() != aDF.Sz() || aRectFile.P0() != cPt2di(0,0))
144-
{
177+
if (aRectFile.Sz() != mDataFile->Sz() || aRectFile.P0() != cPt2di(0, 0))
145178
MMVII_INTERNAL_ERROR("GDAL write: image file created with CreateOnWrite() must be fully written on first write (" + aName + ")");
146-
}
147-
// Delayed file creation, Dataset can be created in memory depending of file format driver
148-
mGdalDataset = cGdalApi::CreateDataset(aDF, &notUpdatable);
149-
if (notUpdatable) {
150-
cGdalApi::SetCreatedNoUpdate(*mDataFile);
151-
} else {
152-
cGdalApi::SetCreated(*mDataFile);
153-
}
179+
180+
mGdalDataset = cGdalApi::CreateDataset(*mDataFile, &notUpdatable);
181+
notUpdatable ? cGdalApi::SetCreatedNoUpdate(*mDataFile) : cGdalApi::SetCreated(*mDataFile);
154182
}
155-
else if (mDataFile->IsCreatedNoUpdate() && aMode == IoMode::Write)
183+
else if (mDataFile->IsCreatedNoUpdate())
156184
{
157-
if (aRectFile.Sz() != aDF.Sz() || aRectFile.P0() != cPt2di(0,0))
158-
{
185+
if (aRectFile.Sz() != mDataFile->Sz() || aRectFile.P0() != cPt2di(0, 0))
159186
MMVII_INTERNAL_ERROR("GDAL write: this image file format must be fully written on each write (" + aName + ")");
160-
}
161-
mGdalDataset = cGdalApi::CreateDataset(aDF, &notUpdatable);
187+
188+
mGdalDataset = cGdalApi::CreateDataset(*mDataFile, &notUpdatable);
162189
}
163190
else
164191
{
165-
mGdalDataset = cGdalApi::OpenDataset(aDF.Name(), aMode==IoMode::Read ? GA_ReadOnly : GA_Update, cGdalApi::eOnError::RaiseError);
192+
mGdalDataset = cGdalApi::OpenDataset(aName, GA_Update, cGdalApi::eOnError::RaiseError);
166193
}
167194

168-
if (aMode == IoMode::Read) {
169-
if (mGdalNbChan == mNbImg && mNbImg != 0) {
170-
GdalReadNtoN(aVecImV2,aRectIm,aRectFile,aDyn); // file N -> N img channels
171-
} else if (mGdalNbChan == 1 && mNbImg != 0) {
172-
GdalRead1toN(aVecImV2,aRectIm,aRectFile,aDyn); // file 1 -> N img channels
173-
} else if (mGdalNbChan != 0 && mNbImg == 1) {
174-
GdalReadNto1(aVecImV2,aRectIm,aRectFile,aDyn); // file N -> 1 img channels
175-
} else {
176-
MMVII_INTERNAL_ERROR("Gdal read: Images vector size: " + std::to_string(mNbImg) + ", file channels: " + std::to_string(mGdalNbChan) + " (" + aName + ")");
177-
}
178-
cGdalApi::CloseDataset(mGdalDataset);
179-
} else {
180-
FileLock gdalLock(aName);
181-
// if (! notUpdatable)
182-
// gdalLock.lock(aName);
183-
if (mGdalNbChan == mNbImg && mNbImg != 0) {
184-
GdalWriteNtoN(aVecImV2,aRectIm,aRectFile,aDyn); // img N -> N file channels
185-
} else if (mGdalNbChan != 0 && mNbImg == 1) {
186-
GdalWrite1toN(aVecImV2,aRectIm,aRectFile,aDyn); // img 1 -> N file channels
187-
} else {
188-
MMVII_INTERNAL_ERROR("Gdal write: Images vector size: " + std::to_string(mNbImg) + ", file channels: " + std::to_string(mGdalNbChan) + " (" + aName + ")");
189-
}
195+
if (mGdalNbChan == mNbImg && mNbImg != 0)
196+
GdalWriteNtoN(aVecImV2, aRectIm, aRectFile, aDyn); // img N -> N file channels
197+
else if (mGdalNbChan != 0 && mNbImg == 1)
198+
GdalWrite1toN(aVecImV2, aRectIm, aRectFile, aDyn); // img 1 -> N file channels
199+
else
200+
MMVII_INTERNAL_ERROR("Gdal write: Images vector size: " + std::to_string(mNbImg) + ", file channels: " + std::to_string(mGdalNbChan) + " (" + aName + ")");
190201

191-
if (notUpdatable) {
192-
// Copy image from memory to file if image file driver needs it
193-
remove(aName.c_str());
194-
auto aGdalDriver = cGdalApi::GetDriver(aName);
195-
auto aGdalOptions = cGdalApi::GetCreateOptions(aGdalDriver, mDataFile->CreateOptions());
196-
remove(aName.c_str());
197-
auto mFinalDataset = aGdalDriver->CreateCopy(aName.c_str(), mGdalDataset, FALSE, aGdalOptions.List(), NULL, NULL);
198-
cGdalApi::CloseDataset(mFinalDataset);
199-
}
200-
cGdalApi::CloseDataset(mGdalDataset);
202+
if (notUpdatable)
203+
{
204+
// Copy image from memory to file if image file driver needs it
205+
auto aGdalDriver = cGdalApi::GetDriver(aName);
206+
auto aGdalOptions = cGdalApi::GetCreateOptions(aGdalDriver, mDataFile->CreateOptions());
207+
remove(aName.c_str());
208+
auto mFinalDataset = aGdalDriver->CreateCopy(aName.c_str(), mGdalDataset, FALSE, aGdalOptions.List(), NULL, NULL);
209+
cGdalApi::CloseDataset(mFinalDataset);
201210
}
211+
212+
cGdalApi::CloseDataset(mGdalDataset);
202213
}
203214

204-
private:
205215
// Helper class: manage N (1 by default) image buffers read from/write to file with GDAL
206216
// "Inherits" from outer class (GdalIO) the template parameter "TypeFile"
207217
class GDalBuffer
@@ -213,11 +223,6 @@ class GdalIO {
213223
{
214224
auto aSize = sizeof(TypeFile)*aRectIm.Sz().x()*aRectIm.Sz().y();
215225
std::generate(mBuffer.begin(), mBuffer.end(),[aSize](){return static_cast<TypeFile*>(cMemManager::Calloc(1,aSize));});
216-
/* for (auto& aBuf : mBuffer)
217-
{
218-
aBuf = static_cast<TypeFile*>(cMemManager::Calloc(1,aSize));
219-
}
220-
*/
221226
}
222227
~GDalBuffer()
223228
{

0 commit comments

Comments
 (0)