Move Ice.Admin creation to createObjectAdapter in C++ #2653
Conversation
| @@ -1577,20 +1577,6 @@ IceInternal::Instance::finishSetup(int& argc, const char* argv[], const Ice::Com | |||
| { | |||
| pluginManagerImpl->initializePlugins(); | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
I believe this is/was the only "async" activity in Ice::initialize.
|
|
||
| adapters = _adapters; | ||
|
|
||
| _instance = nullptr; |
There was a problem hiding this comment.
Refactored this code to avoid calling instance->getAdmin() with _mutex locked. (even putting aside lock-acquisition order concerns, getAdmin should be async and therefore should not be called within a mutex lock).
| @@ -59,7 +59,6 @@ | |||
| <object identity="${server}" type="::Test::TestIntf2"/> | |||
| </adapter> | |||
| <property name="Identity" value="${replicaGroup}"/> | |||
| <property name="Ice.Admin.DelayCreation" value="1"/> | |||
There was a problem hiding this comment.
This setting was apparently not needed.
| @@ -55,6 +55,9 @@ Sub::run(int argc, char** argv) | |||
| throw invalid_argument(os.str()); | |||
| } | |||
|
|
|||
| // With "mx", this creates the Ice.Admin object adapter. | |||
| auto adapter = communicator->createObjectAdapterWithEndpoints("SingleAdapter", "default"); | |||
There was a problem hiding this comment.
Moved here because one test (most likely by accident) relies on a "Xxx ready" getting printed before failing to connect to the topic manager. Failing before "Xxx ready" => test fails.
| // this object adapter, as the endpoint(s) for this object adapter | ||
| // will most likely need to be firewalled for security reasons. | ||
| // | ||
| ObjectAdapterPtr adapter; |
There was a problem hiding this comment.
Removed deprecated IceBox.ServiceManager OA. As far as I can tell, we didn't test it anywhere.
| string managerProxy; | ||
| if (properties->getIceProperty("Ice.Default.Locator").empty()) | ||
| { | ||
| string managerEndpoints = properties->getIceProperty("IceBox.ServiceManager.Endpoints"); |
There was a problem hiding this comment.
Removed this deprecated code too.
| @@ -238,23 +238,6 @@ generated from the section label. | |||
| <suffix name="MessageSizeMax" /> | |||
| </class> | |||
|
|
|||
| <class name="deprecatedthreadpool" prefix-only="true"> | |||
There was a problem hiding this comment.
deprecatedthreadpool and deprecatedobjectadapter were used only by the deprecated IceBox.ServiceManager OA.
|
As Joe pointed out, a big issue with this PR is plugin initialization can create and activate object adapters. So moving |
This PR moves the creation and activation of the Ice.Admin object adapter from
Ice::initializetoCommunicator::createObjectAdapterin C++.Ultimate goal:
I want to keep initialize synchronous, and both Communicator::createObjectAdapter/ObjectAdapter::activate should be asynchronous. See these API in TypeScript and #2624.
I would also like to eliminate Ice.Admin.DelayCreation. Now that the creation of Ice.Admin is always "delayed" until the application creates an OA, there should be no need for this property.
However, we rely on this property for the IceGrid/replicaGroup test. I don't understand why, and replacing it by IceGrid.Admin.Enabled=0 didn't help.
See
ice/cpp/test/IceGrid/replicaGroup/application.xml
Line 74 in 7296c2d