-
Type:
Task
-
Resolution: Fixed
-
Priority:
Major - P3
-
Affects Version/s: None
-
Component/s: None
-
Query Integration
-
Fully Compatible
-
None
-
3
-
TBD
-
None
-
None
-
None
-
None
-
None
-
None
-
None
The Extensions API Host adapter classes leverage a helper Handle class to abstract away access to the underlying public API point and vtable provided by the extension.
While the handle does a good job of checking the validity of the pointer and vtable, it does not provide any type of nullptr checks against individual entries in a vtable. This means that adapters are responsible for checking that individual vtable entries are not null before accessing them.
In our C++ test extensions, the C++ SDK we provide ensures that no vtable entries are null, since the boundary object that the C++ sdk provides across the boundary always populates the entries with corresponding callable functions. However, we are not going to be shipping the C++ SDK for the foreseeable future, and even if we were, we can't rely on any SDK implementation as a guarantee that individual vtable entries are not null, since there is no guarantee that an extension developer actually used the SDK classes to implement their extension.
As part of this ticket, make sure that all host adapter classes assert that the vtable entries it expects to be initialized (i.e not null pointers) are in fact not null pointers. It is important to note that there is also no guarantee that an extension would have populated an uninitialized vtable entry with a nullptr, as this is typically language specific behaviour. However, performing this check on the host adapter is still a safer approach than not checking the pointers at all before forwarding a function call.
As part of this ticket, add a new virtual method on the handle class, named assertVTableConstraints(). This method will be called every time the handle's pointer is updated:
template <typename T, bool IsOwned>class Handle { public: ... ... protected: // This method must be implemented by all derivations of Handle. It must assert that all the vtable entries it expects to be non-null are valid. It is called whenever the handle's pointer is updated. This allows us to freely forward calls to the vtable on demand without having to check the vtable entry for null everytime we call a method on it. // Here we provide a base implementation for the OwnedHandle, since we must have a destroy method in the vtable. virtual void assertVTableConstraints() { if constexpr(isOwned) { const auto& vtbl = vtable(); tassert(...., vtbl.destroy != nullptr); } }; };
For example:
class ExtensionAggregationStageDescriptorHandle : public OwnedHandle<::MongoExtensionAggregationStageDescriptor> { public: protected: void assertVTableConstraints() override { const auto& vtable = vtable(); tassert(...., vtable.get_name != nullptr); tassert(...., vtable.get_type != nullptr); tassert(...., vtable.parse != nullptr); } };
We can further expand this concept once we want to handle multi-versioned extension scenarios, where we might want to relax some conditions if an extension is not yet on the latest SDK release, and there may be some unimplemented functions.
Link to original thread on github PR: https://github.com/10gen/mongo/pull/39882#discussion_r2271824590