diff options
author | Michael Stahl <michael.stahl@allotropia.de> | 2023-11-21 14:48:03 +0100 |
---|---|---|
committer | Michael Stahl <michael.stahl@allotropia.de> | 2023-11-24 10:22:49 +0100 |
commit | 7d7d84c07b1ed01a5cd51b43c5712eab19d3f729 (patch) | |
tree | eed115c22475e4d58637aa02b1a6f7de82933ec9 | |
parent | a596070f8ac11ed0cd22baf55704037a6b8d9c4d (diff) |
sd: remote: disentangle class RemoteServer
RemoteServer was both an object and a thread instantiated for the IP
server, and a bunch of static methods and data used by all servers.
Split out a class IPRemoteServer.
Move Bluetooth setup to SdDLL::RegisterRemotes().
Also the BluetoothServer was accessing RemoteServer::sCommunicators but
never locked the corresponding RemoteServer::sDataMutex.
Change-Id: I3ff39e68e619af1e91697124a1352b5f20350a22
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159785
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>
-rw-r--r-- | sd/source/ui/dlg/RemoteDialog.cxx | 2 | ||||
-rw-r--r-- | sd/source/ui/dlg/RemoteDialogClientBox.cxx | 4 | ||||
-rw-r--r-- | sd/source/ui/inc/RemoteServer.hxx | 41 | ||||
-rw-r--r-- | sd/source/ui/remotecontrol/BluetoothServer.cxx | 20 | ||||
-rw-r--r-- | sd/source/ui/remotecontrol/Server.cxx | 58 |
5 files changed, 74 insertions, 51 deletions
diff --git a/sd/source/ui/dlg/RemoteDialog.cxx b/sd/source/ui/dlg/RemoteDialog.cxx index 411e4ea621bc..6a66c2acfda2 100644 --- a/sd/source/ui/dlg/RemoteDialog.cxx +++ b/sd/source/ui/dlg/RemoteDialog.cxx @@ -31,7 +31,7 @@ IMPL_LINK_NOARG(RemoteDialog, HandleConnectButton, weld::Button&, void) if (!xEntry) return; OUString aPin = xEntry->m_xPinBox->get_text(); - if (RemoteServer::connectClient(xEntry->m_xClientInfo, aPin)) + if (IPRemoteServer::connectClient(xEntry->m_xClientInfo, aPin)) m_xDialog->response(RET_OK); #endif } diff --git a/sd/source/ui/dlg/RemoteDialogClientBox.cxx b/sd/source/ui/dlg/RemoteDialogClientBox.cxx index 3d5979c4317d..44e2c6f65064 100644 --- a/sd/source/ui/dlg/RemoteDialogClientBox.cxx +++ b/sd/source/ui/dlg/RemoteDialogClientBox.cxx @@ -104,7 +104,7 @@ void ClientBox::populateEntries() #ifdef ENABLE_SDREMOTE RemoteServer::ensureDiscoverable(); - std::vector< std::shared_ptr< ClientInfo > > aClients( RemoteServer::getClients() ); + std::vector<std::shared_ptr<ClientInfo>> const aClients(IPRemoteServer::getClients()); for ( const auto& rxClient : aClients ) { @@ -117,7 +117,7 @@ void ClientBox::populateEntries() IMPL_LINK_NOARG(ClientBoxEntry, DeauthoriseHdl, weld::Button&, void) { #ifdef ENABLE_SDREMOTE - RemoteServer::deauthoriseClient(m_xClientInfo); + IPRemoteServer::deauthoriseClient(m_xClientInfo); #endif m_pClientBox->populateEntries(); } diff --git a/sd/source/ui/inc/RemoteServer.hxx b/sd/source/ui/inc/RemoteServer.hxx index aa42ffb7d03e..70eed9aa8d52 100644 --- a/sd/source/ui/inc/RemoteServer.hxx +++ b/sd/source/ui/inc/RemoteServer.hxx @@ -47,23 +47,14 @@ namespace sd struct ClientInfoInternal; - class RemoteServer final : public salhelper::Thread + class RemoteServer final { public: - // Internal setup - static void setup(); - // For slideshowimpl to inform us. static void presentationStarted( const css::uno::Reference< css::presentation::XSlideShowController > &rController ); static void presentationStopped(); - // For the control dialog - SD_DLLPUBLIC static std::vector< std::shared_ptr< ClientInfo > > getClients(); - SD_DLLPUBLIC static bool connectClient( const std::shared_ptr< ClientInfo >& pClient, - std::u16string_view aPin ); - SD_DLLPUBLIC static void deauthoriseClient( const std::shared_ptr< ClientInfo >& pClient ); - /// ensure that discoverability (eg. for Bluetooth) is enabled SD_DLLPUBLIC static void ensureDiscoverable(); /// restore the state of discoverability from before ensureDiscoverable @@ -71,18 +62,36 @@ namespace sd // For the communicator static void removeCommunicator( Communicator const * pCommunicator ); - private: - RemoteServer(); - virtual ~RemoteServer() override; - static RemoteServer *spServer; + //private: + // these are public because 3 classes and a function need access static ::osl::Mutex sDataMutex; static ::std::vector<Communicator*> sCommunicators; - osl::AcceptorSocket mSocket; + }; - ::std::vector< std::shared_ptr< ClientInfoInternal > > mAvailableClients; + class IPRemoteServer final : public salhelper::Thread + { + public: + // Internal setup + static void setup(); + + // For the control dialog + SD_DLLPUBLIC static std::vector<std::shared_ptr<ClientInfo>> getClients(); + SD_DLLPUBLIC static bool connectClient(const std::shared_ptr<ClientInfo>& pClient, + std::u16string_view aPin); + SD_DLLPUBLIC static void deauthoriseClient(const std::shared_ptr<ClientInfo>& pClient); void execute() override; void handleAcceptedConnection( BufferedStreamSocket *pSocket ) ; + + private: + IPRemoteServer(); + virtual ~IPRemoteServer() override; + + osl::AcceptorSocket mSocket; + ::std::vector<std::shared_ptr<ClientInfoInternal>> mAvailableClients; + + friend class RemoteServer; + static IPRemoteServer *spServer; }; } diff --git a/sd/source/ui/remotecontrol/BluetoothServer.cxx b/sd/source/ui/remotecontrol/BluetoothServer.cxx index fc3eeff54255..0c568556b56a 100644 --- a/sd/source/ui/remotecontrol/BluetoothServer.cxx +++ b/sd/source/ui/remotecontrol/BluetoothServer.cxx @@ -52,6 +52,9 @@ #endif #include "Communicator.hxx" +#include <RemoteServer.hxx> + +#include <osl/mutex.hxx> using namespace sd; @@ -572,6 +575,7 @@ void incomingCallback( void *userRefCon, void BluetoothServer::addCommunicator( Communicator* pCommunicator ) { + ::osl::MutexGuard aGuard(RemoteServer::sDataMutex); mpCommunicators->push_back( pCommunicator ); } @@ -922,7 +926,10 @@ static DBusHandlerResult ProfileMessageFunction SAL_INFO( "sdremote.bluetooth", "connection accepted " << nDescriptor); Communicator* pCommunicator = new Communicator( std::make_unique<BufferedStreamSocket>( nDescriptor ) ); - pCommunicators->push_back( pCommunicator ); + { + ::osl::MutexGuard aGuard(RemoteServer::sDataMutex); + pCommunicators->push_back( pCommunicator ); + } pCommunicator->launch(); } @@ -1149,6 +1156,7 @@ void BluetoothServer::doRestoreDiscoverable() // re-bind to the same port number it appears. void BluetoothServer::cleanupCommunicators() { + ::osl::MutexGuard aGuard(RemoteServer::sDataMutex); for (auto& rpCommunicator : *mpCommunicators) rpCommunicator->forceClose(); // the hope is that all the threads then terminate cleanly and @@ -1286,7 +1294,10 @@ void SAL_CALL BluetoothServer::run() } else { SAL_INFO( "sdremote.bluetooth", "connection accepted " << nClient ); Communicator* pCommunicator = new Communicator( std::make_unique<BufferedStreamSocket>( nClient ) ); - mpCommunicators->push_back( pCommunicator ); + { + ::osl::MutexGuard aGuard(RemoteServer::sDataMutex); + mpCommunicators->push_back( pCommunicator ); + } pCommunicator->launch(); } } @@ -1383,7 +1394,10 @@ void SAL_CALL BluetoothServer::run() return; } else { Communicator* pCommunicator = new Communicator( std::make_unique<BufferedStreamSocket>( socket) ); - mpCommunicators->push_back( pCommunicator ); + { + ::osl::MutexGuard aGuard(RemoteServer::sDataMutex); + mpCommunicators->push_back( pCommunicator ); + } pCommunicator->launch(); } } diff --git a/sd/source/ui/remotecontrol/Server.cxx b/sd/source/ui/remotecontrol/Server.cxx index 962afbe5f9c2..3f77758bf728 100644 --- a/sd/source/ui/remotecontrol/Server.cxx +++ b/sd/source/ui/remotecontrol/Server.cxx @@ -63,19 +63,19 @@ namespace sd { }; } -RemoteServer::RemoteServer() : - Thread( "RemoteServerThread" ) +IPRemoteServer::IPRemoteServer() + : Thread("IPRemoteServerThread") { - SAL_INFO( "sdremote", "Instantiated RemoteServer" ); + SAL_INFO("sdremote", "Instantiated IPRemoteServer"); } -RemoteServer::~RemoteServer() +IPRemoteServer::~IPRemoteServer() { } -void RemoteServer::execute() +void IPRemoteServer::execute() { - SAL_INFO( "sdremote", "RemoteServer::execute called" ); + SAL_INFO("sdremote", "IPRemoteServer::execute called"); osl::SocketAddr aAddr( "0.0.0.0", PORT ); if ( !mSocket.bind( aAddr ) ) { @@ -103,11 +103,11 @@ void RemoteServer::execute() BufferedStreamSocket *pSocket = new BufferedStreamSocket( aSocket); handleAcceptedConnection( pSocket ); } - SAL_INFO( "sdremote", "shutting down RemoteServer" ); + SAL_INFO("sdremote", "shutting down IPRemoteServer"); spServer = nullptr; // Object is destroyed when Thread::execute() ends. } -void RemoteServer::handleAcceptedConnection( BufferedStreamSocket *pSocket ) +void IPRemoteServer::handleAcceptedConnection( BufferedStreamSocket *pSocket ) { OString aLine; if ( ! ( pSocket->readLine( aLine) @@ -142,7 +142,7 @@ void RemoteServer::handleAcceptedConnection( BufferedStreamSocket *pSocket ) } while ( aLine.getLength() > 0 ); - MutexGuard aGuard( sDataMutex ); + MutexGuard aGuard(RemoteServer::sDataMutex); std::shared_ptr< ClientInfoInternal > pClient = std::make_shared<ClientInfoInternal>( OStringToOUString( aName, RTL_TEXTENCODING_UTF8 ), @@ -175,27 +175,23 @@ void RemoteServer::handleAcceptedConnection( BufferedStreamSocket *pSocket ) strlen( "LO_SERVER_VALIDATING_PIN\n\n" ) ); } -RemoteServer *sd::RemoteServer::spServer = nullptr; +IPRemoteServer *sd::IPRemoteServer::spServer = nullptr; ::osl::Mutex sd::RemoteServer::sDataMutex; ::std::vector<Communicator*> sd::RemoteServer::sCommunicators; -void RemoteServer::setup() +void IPRemoteServer::setup() { if (spServer) return; - spServer = new RemoteServer(); + spServer = new IPRemoteServer(); spServer->launch(); - -#ifdef ENABLE_SDREMOTE_BLUETOOTH - sd::BluetoothServer::setup( &sCommunicators ); -#endif } void RemoteServer::presentationStarted( const css::uno::Reference< css::presentation::XSlideShowController > &rController ) { - if ( !spServer ) + if (!IPRemoteServer::spServer) return; MutexGuard aGuard( sDataMutex ); for ( const auto& rpCommunicator : sCommunicators ) @@ -205,7 +201,7 @@ void RemoteServer::presentationStarted( const css::uno::Reference< } void RemoteServer::presentationStopped() { - if ( !spServer ) + if (!IPRemoteServer::spServer) return; MutexGuard aGuard( sDataMutex ); for ( const auto& rpCommunicator : sCommunicators ) @@ -216,7 +212,7 @@ void RemoteServer::presentationStopped() void RemoteServer::removeCommunicator( Communicator const * mCommunicator ) { - if ( !spServer ) + if (!IPRemoteServer::spServer) return; MutexGuard aGuard( sDataMutex ); auto aIt = std::find(sCommunicators.begin(), sCommunicators.end(), mCommunicator); @@ -224,13 +220,13 @@ void RemoteServer::removeCommunicator( Communicator const * mCommunicator ) sCommunicators.erase( aIt ); } -std::vector< std::shared_ptr< ClientInfo > > RemoteServer::getClients() +std::vector<std::shared_ptr<ClientInfo>> IPRemoteServer::getClients() { - SAL_INFO( "sdremote", "RemoteServer::getClients() called" ); + SAL_INFO( "sdremote", "IPRemoteServer::getClients() called" ); std::vector< std::shared_ptr< ClientInfo > > aClients; if ( spServer ) { - MutexGuard aGuard( sDataMutex ); + MutexGuard aGuard(RemoteServer::sDataMutex); aClients.assign( spServer->mAvailableClients.begin(), spServer->mAvailableClients.end() ); } @@ -256,9 +252,9 @@ std::vector< std::shared_ptr< ClientInfo > > RemoteServer::getClients() return aClients; } -bool RemoteServer::connectClient( const std::shared_ptr< ClientInfo >& pClient, std::u16string_view aPin ) +bool IPRemoteServer::connectClient(const std::shared_ptr<ClientInfo>& pClient, std::u16string_view aPin) { - SAL_INFO( "sdremote", "RemoteServer::connectClient called" ); + SAL_INFO("sdremote", "IPRemoteServer::connectClient called"); if ( !spServer ) return false; @@ -293,9 +289,9 @@ bool RemoteServer::connectClient( const std::shared_ptr< ClientInfo >& pClient, } Communicator* pCommunicator = new Communicator( std::unique_ptr<IBluetoothSocket>(apClient->mpStreamSocket) ); - MutexGuard aGuard( sDataMutex ); + MutexGuard aGuard(RemoteServer::sDataMutex); - sCommunicators.push_back( pCommunicator ); + RemoteServer::sCommunicators.push_back( pCommunicator ); auto aIt = std::find(spServer->mAvailableClients.begin(), spServer->mAvailableClients.end(), pClient); if (aIt != spServer->mAvailableClients.end()) @@ -309,13 +305,13 @@ bool RemoteServer::connectClient( const std::shared_ptr< ClientInfo >& pClient, } } -void RemoteServer::deauthoriseClient( const std::shared_ptr< ClientInfo >& pClient ) +void IPRemoteServer::deauthoriseClient(const std::shared_ptr<ClientInfo>& pClient) { // TODO: we probably want to forcefully disconnect at this point too? // But possibly via a separate function to allow just disconnecting from // the UI. - SAL_INFO( "sdremote", "RemoteServer::deauthoriseClient called" ); + SAL_INFO("sdremote", "IPRemoteServer::deauthoriseClient called"); if ( !pClient->mbIsAlreadyAuthorised ) // We can't remove unauthorised clients from the authorised list... @@ -352,7 +348,11 @@ void SdDLL::RegisterRemotes() if ( !officecfg::Office::Impress::Misc::Start::EnableSdremote::get() ) return; - sd::RemoteServer::setup(); +#ifdef ENABLE_SDREMOTE_BLUETOOTH + sd::BluetoothServer::setup( &RemoteServer::sCommunicators ); +#endif + + sd::IPRemoteServer::setup(); sd::DiscoveryService::setup(); } |