Skip to content

Commit afc6293

Browse files
ktfktf
committed
DPL: complain when using the wrong context
1 parent 5c45332 commit afc6293

3 files changed

Lines changed: 51 additions & 24 deletions

File tree

‎Framework/Core/include/Framework/ServiceRegistry.h‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,25 @@ struct ServiceRegistry {
9999
int32_t index = -1;
100100
};
101101

102-
// Metadata about the service
102+
// Metadata about the service. This
103+
// might be interesting for debugging purposes.
104+
// however it's not used to uniquely identify
105+
// the service.
103106
struct Meta {
104107
ServiceKind kind = ServiceKind::Serial;
105-
Salt salt = {0, 0};
108+
char const* name = nullptr;
109+
};
110+
111+
// Unique identifier for a service.
112+
// While we use the salted hash to find the bucket
113+
// in the hashmap, the service can be uniquely identified
114+
// only by this 64 bit value.
115+
union Key {
116+
struct Store {
117+
ServiceTypeHash typeHash;
118+
Salt salt;
119+
} store;
120+
uint64_t value = 0;
106121
};
107122

108123
/// The maximum distance a entry can be from the optimal slot.
@@ -284,7 +299,7 @@ struct ServiceRegistry {
284299
}
285300

286301
mutable std::vector<ServiceSpec> mSpecs;
287-
mutable std::array<std::atomic<uint32_t>, MAX_SERVICES + MAX_DISTANCE> mServicesKey;
302+
mutable std::array<std::atomic<Key>, MAX_SERVICES + MAX_DISTANCE> mServicesKey;
288303
mutable std::array<void*, MAX_SERVICES + MAX_DISTANCE> mServicesValue;
289304
mutable std::array<Meta, MAX_SERVICES + MAX_DISTANCE> mServicesMeta;
290305
mutable std::array<std::atomic<bool>, MAX_SERVICES + MAX_DISTANCE> mServicesBooked;

‎Framework/Core/src/ServiceRegistry.cxx‎

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ ServiceRegistry& ServiceRegistry::operator=(ServiceRegistry const& other)
4545
ServiceRegistry::ServiceRegistry()
4646
{
4747
for (size_t i = 0; i < MAX_SERVICES; ++i) {
48-
mServicesKey[i].store(0L);
48+
mServicesKey[i].store({0L, 0L});
4949
}
5050

5151
mServicesValue.fill(nullptr);
@@ -79,8 +79,8 @@ void ServiceRegistry::registerService(ServiceTypeHash typeHash, void* service, S
7979
if (mServicesBooked[i + index.index].compare_exchange_strong(expected, true,
8080
std::memory_order_seq_cst)) {
8181
mServicesValue[i + index.index] = service;
82-
mServicesMeta[i + index.index] = Meta{kind, salt};
83-
mServicesKey[i + index.index] = typeHash.hash;
82+
mServicesMeta[i + index.index] = Meta{kind, name ? strdup(name) : nullptr};
83+
mServicesKey[i + index.index] = Key{.store = {.typeHash = typeHash, .salt = salt}};
8484
std::atomic_thread_fence(std::memory_order_release);
8585
return;
8686
}
@@ -270,7 +270,7 @@ int ServiceRegistry::getPos(ServiceTypeHash typeHash, Salt salt) const
270270
InstanceId instanceId = instanceFromTypeSalt(typeHash, salt);
271271
Index index = indexFromInstance(instanceId);
272272
for (uint8_t i = 0; i < MAX_DISTANCE; ++i) {
273-
if (mServicesKey[i + index.index].load() == typeHash.hash) {
273+
if (mServicesKey[i + index.index].load().value == Key{typeHash.hash, salt}.value) {
274274
return i + index.index;
275275
}
276276
}
@@ -279,30 +279,36 @@ int ServiceRegistry::getPos(ServiceTypeHash typeHash, Salt salt) const
279279

280280
void* ServiceRegistry::get(ServiceTypeHash typeHash, Salt salt, ServiceKind kind, char const* name) const
281281
{
282+
// Cannot find a stream service using a global salt.
283+
if (salt.context.streamId == GLOBAL_CONTEXT_SALT.value && kind == ServiceKind::Stream) {
284+
throwError(runtime_error("Cannot find a global service using a stream salt."));
285+
}
282286
// Look for the service. If found, return it.
283287
// Notice how due to threading issues, we might
284288
// find it with getPos, but the value can still
285289
// be nullptr.
286290
auto pos = getPos(typeHash, salt);
287-
if (pos != -1 && mServicesMeta[pos].kind == ServiceKind::Stream && mServicesMeta[pos].salt.value != salt.value) {
288-
throwError(runtime_error_f("Inconsistent registry for thread %d. Expected %d", salt.context.streamId, mServicesMeta[pos].salt.context.streamId));
291+
// If we are here it means we never registered a
292+
// service for the 0 thread (i.e. the main thread).
293+
if (pos != -1 && mServicesMeta[pos].kind == ServiceKind::Stream && mServicesKey[pos].load().store.salt.value != salt.value) {
294+
throwError(runtime_error_f("Inconsistent registry for thread %d. Expected %d", salt.context.streamId, mServicesKey[pos].load().store.salt.context.streamId));
289295
O2_BUILTIN_UNREACHABLE();
290296
}
291297

292-
bool isStream = mServicesMeta[pos].kind == ServiceKind::DataProcessorStream || mServicesMeta[pos].kind == ServiceKind::DeviceStream;
293-
bool isDataProcessor = mServicesMeta[pos].kind == ServiceKind::DataProcessorStream || mServicesMeta[pos].kind == ServiceKind::DataProcessorGlobal || mServicesMeta[pos].kind == ServiceKind::DataProcessorSerial;
298+
if (pos != -1) {
299+
bool isStream = mServicesMeta[pos].kind == ServiceKind::DataProcessorStream || mServicesMeta[pos].kind == ServiceKind::DeviceStream;
300+
bool isDataProcessor = mServicesMeta[pos].kind == ServiceKind::DataProcessorStream || mServicesMeta[pos].kind == ServiceKind::DataProcessorGlobal || mServicesMeta[pos].kind == ServiceKind::DataProcessorSerial;
294301

295-
if (pos != -1 && isStream && salt.context.streamId <= 0) {
296-
throwError(runtime_error_f("A stream service cannot be retrieved from a non stream salt %d", salt.context.streamId));
297-
O2_BUILTIN_UNREACHABLE();
298-
}
299-
300-
if (pos != -1 && isDataProcessor && salt.context.dataProcessorId < 0) {
301-
throwError(runtime_error_f("A data processor service cannot be retrieved from a non dataprocessor context %d", salt.context.dataProcessorId));
302-
O2_BUILTIN_UNREACHABLE();
303-
}
302+
if (isStream && salt.context.streamId <= 0) {
303+
throwError(runtime_error_f("A stream service cannot be retrieved from a non stream salt %d", salt.context.streamId));
304+
O2_BUILTIN_UNREACHABLE();
305+
}
306+
307+
if (isDataProcessor && salt.context.dataProcessorId < 0) {
308+
throwError(runtime_error_f("A data processor service cannot be retrieved from a non dataprocessor context %d", salt.context.dataProcessorId));
309+
O2_BUILTIN_UNREACHABLE();
310+
}
304311

305-
if (pos != -1) {
306312
mServicesKey[pos].load();
307313
std::atomic_thread_fence(std::memory_order_acquire);
308314
void* ptr = mServicesValue[pos];
@@ -328,8 +334,6 @@ void* ServiceRegistry::get(ServiceTypeHash typeHash, Salt salt, ServiceKind kind
328334
throwError(runtime_error_f("Unable to find requested service %s", name));
329335
}
330336
}
331-
// If we are here it means we never registered a
332-
// service for the 0 thread (i.e. the main thread).
333337
return nullptr;
334338
}
335339

‎Framework/Core/test/test_Services.cxx‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ BOOST_AUTO_TEST_CASE(TestStreamServices)
161161
DummyService t0{0};
162162
DummyService t1{1};
163163
DummyService t2{2};
164+
DummyService t2_d1{2};
164165
/// We register it pretending to be on thread 0
165166
registry.registerService({TypeIdHelpers::uniqueId<DummyService>()}, &t0, ServiceKind::Stream, ServiceRegistry::Salt{ServiceRegistry::Context{1, 0}});
166167
registry.registerService({TypeIdHelpers::uniqueId<DummyService>()}, &t1, ServiceKind::Stream, ServiceRegistry::Salt{ServiceRegistry::Context{2, 0}});
@@ -172,9 +173,16 @@ BOOST_AUTO_TEST_CASE(TestStreamServices)
172173
BOOST_CHECK_EQUAL(tt0->threadId, 0);
173174
BOOST_CHECK_EQUAL(tt1->threadId, 1);
174175
BOOST_CHECK_EQUAL(tt2->threadId, 2);
176+
// Check that Context{1,1} throws, because we registerd it for a different data processor.
177+
BOOST_CHECK_THROW(registry.get({TypeIdHelpers::uniqueId<DummyService>()}, salt_1_1, ServiceKind::Stream), RuntimeErrorRef);
175178
// Check that Context{0,0} throws.
176179
BOOST_CHECK_THROW(registry.get({TypeIdHelpers::uniqueId<DummyService>()}, salt_0, ServiceKind::Stream), RuntimeErrorRef);
177-
// Check that Context{1,1} throws, because we registerd it for a different data processor.
180+
181+
registry.registerService({TypeIdHelpers::uniqueId<DummyService>()}, &t2_d1, ServiceKind::Stream, ServiceRegistry::Salt{ServiceRegistry::Context{3, 1}});
182+
183+
auto tt2_dp1 = reinterpret_cast<DummyService*>(registry.get({TypeIdHelpers::uniqueId<DummyService>()}, ServiceRegistry::Salt{3, 1}, ServiceKind::Stream));
184+
BOOST_CHECK_EQUAL(tt2_dp1->threadId, 2);
185+
178186
BOOST_CHECK_THROW(registry.get({TypeIdHelpers::uniqueId<DummyService>()}, salt_1_1, ServiceKind::Stream), RuntimeErrorRef);
179187
}
180188

0 commit comments

Comments
 (0)