Skip to content

Commit 1b88f1c

Browse files
committed
Get ServiceToken in TStorageFactoryFile
It is possible for ROOT to call methods on different threads. The underlying storage class may need a service in order to work so we need to make sure the service system is running on that thread.
1 parent 5c35891 commit 1b88f1c

File tree

2 files changed

+18
-1
lines changed

2 files changed

+18
-1
lines changed

IOPool/TFileAdaptor/interface/TStorageFactoryFile.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include "Utilities/StorageFactory/interface/IOPosBuffer.h"
1010
#include "FWCore/Utilities/interface/get_underlying_safe.h"
11+
#include "FWCore/ServiceRegistry/interface/ServiceToken.h"
1112

1213
namespace edm::storage {
1314
class Storage;
@@ -58,6 +59,7 @@ class TStorageFactoryFile : public TFile {
5859
TStorageFactoryFile(void);
5960

6061
edm::propagate_const<std::unique_ptr<edm::storage::Storage>> storage_; //< Real underlying storage
62+
edm::ServiceWeakToken token_;
6163
};
6264

6365
#endif // TFILE_ADAPTOR_TSTORAGE_FACTORY_FILE_H

IOPool/TFileAdaptor/src/TStorageFactoryFile.cc

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "Utilities/StorageFactory/interface/StorageAccount.h"
55
#include "Utilities/StorageFactory/interface/StatisticsSenderService.h"
66
#include "FWCore/ServiceRegistry/interface/Service.h"
7+
#include "FWCore/ServiceRegistry/interface/ServiceRegistry.h"
78
#include "FWCore/Utilities/interface/EDMException.h"
89
#include "FWCore/Utilities/interface/ExceptionPropagate.h"
910
#include "ReadRepacker.h"
@@ -99,7 +100,7 @@ static inline StorageAccount::Counter &storageCounter(std::atomic<StorageAccount
99100
return *c.load();
100101
}
101102

102-
TStorageFactoryFile::TStorageFactoryFile(void) : storage_() {
103+
TStorageFactoryFile::TStorageFactoryFile() : storage_(), token_(edm::ServiceRegistry::instance().presentToken()) {
103104
StorageAccount::Stamp stats(storageCounter(s_statsCtor, StorageAccount::Operation::construct));
104105
stats.tick(0);
105106
}
@@ -198,6 +199,7 @@ void TStorageFactoryFile::Initialize(const char *path, Option_t *option /* = ""
198199
}
199200

200201
// Record the statistics.
202+
token_ = edm::ServiceRegistry::instance().presentToken();
201203
try {
202204
edm::Service<edm::storage::StatisticsSenderService> statsService;
203205
if (statsService.isAvailable()) {
@@ -227,6 +229,7 @@ TStorageFactoryFile::~TStorageFactoryFile(void) { Close(); }
227229
Bool_t TStorageFactoryFile::ReadBuffer(char *buf, Long64_t pos, Int_t len) {
228230
// This function needs to be optimized to minimize seeks.
229231
// See TFile::ReadBuffer(char *buf, Long64_t pos, Int_t len) in ROOT 5.27.06.
232+
edm::ServiceRegistry::Operate operate(token_.lock());
230233
Seek(pos);
231234
return ReadBuffer(buf, len);
232235
}
@@ -253,6 +256,8 @@ Bool_t TStorageFactoryFile::ReadBuffer(char *buf, Int_t len) {
253256
// the amount actually passed to read from the storage object.
254257
StorageAccount::Stamp stats(storageCounter(s_statsRead, StorageAccount::Operation::read));
255258

259+
edm::ServiceRegistry::Operate operate(token_.lock());
260+
256261
// If we have a cache, read from there first. This returns 0
257262
// if the block hasn't been prefetched, 1 if it was in cache,
258263
// and 2 if there was an error.
@@ -327,6 +332,8 @@ Bool_t TStorageFactoryFile::ReadBufferAsync(Long64_t off, Int_t len) {
327332

328333
StorageAccount::Stamp stats(storageCounter(s_statsARead, StorageAccount::Operation::readAsync));
329334

335+
edm::ServiceRegistry::Operate operate(token_.lock());
336+
330337
// If asynchronous reading is disabled, bail out now, regardless
331338
// whether the underlying storage supports prefetching. If it is
332339
// forced on, pretend it's on, even if the storage doesn't support
@@ -447,6 +454,8 @@ Bool_t TStorageFactoryFile::ReadBuffers(char *buf, Long64_t *pos, Int_t *len, In
447454
return kTRUE;
448455
}
449456

457+
edm::ServiceRegistry::Operate operate(token_.lock());
458+
450459
// For synchronous reads, we have special logic to optimize the I/O requests
451460
// from ROOT before handing it to the storage.
452461
if (buf) {
@@ -500,6 +509,8 @@ Bool_t TStorageFactoryFile::WriteBuffer(const char *buf, Int_t len) {
500509
return kTRUE;
501510
}
502511

512+
edm::ServiceRegistry::Operate operate(token_.lock());
513+
503514
StorageAccount::Stamp stats(storageCounter(s_statsWrite, StorageAccount::Operation::write));
504515
StorageAccount::Stamp cstats(storageCounter(s_statsCWrite, StorageAccount::Operation::writeViaCache));
505516

@@ -588,13 +599,17 @@ Long64_t TStorageFactoryFile::SysSeek(Int_t /* fd */, Long64_t offset, Int_t whe
588599
StorageAccount::Stamp stats(storageCounter(s_statsSeek, StorageAccount::Operation::seek));
589600
Storage::Relative rel = (whence == SEEK_SET ? Storage::SET : whence == SEEK_CUR ? Storage::CURRENT : Storage::END);
590601

602+
edm::ServiceRegistry::Operate operate(token_.lock());
603+
591604
offset = storage_->position(offset, rel);
592605
stats.tick();
593606
return offset;
594607
}
595608

596609
Int_t TStorageFactoryFile::SysSync(Int_t /* fd */) {
597610
StorageAccount::Stamp stats(storageCounter(s_statsFlush, StorageAccount::Operation::flush));
611+
edm::ServiceRegistry::Operate operate(token_.lock());
612+
598613
storage_->flush();
599614
stats.tick();
600615
return 0;

0 commit comments

Comments
 (0)