Skip to content

Commit faa9871

Browse files
committed
fixup! Ensure cluster cache is properly clean in case of exception in constructor.
Calling non static member from constructor try block handler is UB. Accessing this (witohut dereferencing it) is kind of gray area as (See https://stackoverflow.com/questions/79463018/can-i-access-this-from-contructor-try-block-handler) So the try/catch has been moved to a more classic try/catch block as only a specific part of the contructor can create (and cache) clusters.
1 parent 2a5a571 commit faa9871

File tree

2 files changed

+29
-18
lines changed

2 files changed

+29
-18
lines changed

src/fileimpl.cpp

+27-18
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ class Grouping
188188
{}
189189
#endif
190190

191-
FileImpl::FileImpl(std::shared_ptr<FileCompound> _zimFile) try
191+
FileImpl::FileImpl(std::shared_ptr<FileCompound> _zimFile)
192192
: zimFile(_zimFile),
193193
zimReader(makeFileReader(zimFile)),
194194
direntReader(new DirentReader(zimReader)),
@@ -248,33 +248,42 @@ class Grouping
248248
const_cast<entry_index_t&>(m_endUserEntry) = getCountArticles();
249249
}
250250

251-
auto result = tmp_direntLookup.find('X', "listing/titleOrdered/v1");
252-
if (result.first) {
253-
mp_titleDirentAccessor = getTitleAccessorV1(result.second);
254-
}
255251

256-
if (!mp_titleDirentAccessor) {
257-
if (!header.hasTitleListingV0()) {
258-
throw ZimFileFormatError("Zim file doesn't contain a title ordered index");
252+
// Following code will may create cluster and
253+
try {
254+
auto result = tmp_direntLookup.find('X', "listing/titleOrdered/v1");
255+
if (result.first) {
256+
mp_titleDirentAccessor = getTitleAccessorV1(result.second);
259257
}
260-
offset_t titleOffset(header.getTitleIdxPos());
261-
zsize_t titleSize(sizeof(entry_index_type)*header.getArticleCount());
262-
mp_titleDirentAccessor = getTitleAccessor(titleOffset, titleSize, "Title index table");
263-
const_cast<bool&>(m_hasFrontArticlesIndex) = false;
264-
}
265-
m_byTitleDirentLookup.reset(new ByTitleDirentLookup(mp_titleDirentAccessor.get()));
266258

267-
readMimeTypes();
268-
} catch (...) {
269-
getClusterCache().drop_all([=](const std::tuple<FileImpl*, cluster_index_type>& key) {return std::get<0>(key) == this;});
270-
throw;
259+
if (!mp_titleDirentAccessor) {
260+
if (!header.hasTitleListingV0()) {
261+
throw ZimFileFormatError("Zim file doesn't contain a title ordered index");
262+
}
263+
offset_t titleOffset(header.getTitleIdxPos());
264+
zsize_t titleSize(sizeof(entry_index_type)*header.getArticleCount());
265+
mp_titleDirentAccessor = getTitleAccessor(titleOffset, titleSize, "Title index table");
266+
const_cast<bool&>(m_hasFrontArticlesIndex) = false;
267+
}
268+
m_byTitleDirentLookup.reset(new ByTitleDirentLookup(mp_titleDirentAccessor.get()));
269+
270+
readMimeTypes();
271+
} catch (...) {
272+
dropCachedCluster();
273+
throw;
274+
}
271275
}
272276

273277
FileImpl::~FileImpl() {
274278
// We have to clean the global cache for our clusters.
279+
dropCachedCluster();
280+
}
281+
282+
void FileImpl::dropCachedCluster() const {
275283
getClusterCache().drop_all([=](const std::tuple<FileImpl*, cluster_index_type>& key) {return std::get<0>(key) == this;});
276284
}
277285

286+
278287
std::unique_ptr<IndirectDirentAccessor> FileImpl::getTitleAccessorV1(const entry_index_t idx)
279288
{
280289
auto dirent = mp_pathDirentAccessor->getDirent(idx);

src/fileimpl.h

+2
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,8 @@ namespace zim
166166
explicit FileImpl(std::shared_ptr<FileCompound> zimFile);
167167
FileImpl(std::shared_ptr<FileCompound> zimFile, offset_t offset, zsize_t size);
168168

169+
void dropCachedCluster() const;
170+
169171
std::unique_ptr<IndirectDirentAccessor> getTitleAccessorV1(const entry_index_t idx);
170172
std::unique_ptr<IndirectDirentAccessor> getTitleAccessor(const offset_t offset, const zsize_t size, const std::string& name);
171173

0 commit comments

Comments
 (0)