diff --git a/pvAccessApp/server/beaconEmitter.cpp b/pvAccessApp/server/beaconEmitter.cpp index 4e527b0..0e5c7b9 100644 --- a/pvAccessApp/server/beaconEmitter.cpp +++ b/pvAccessApp/server/beaconEmitter.cpp @@ -57,7 +57,9 @@ BeaconEmitter::BeaconEmitter(Transport::shared_pointer const & transport, const BeaconEmitter::~BeaconEmitter() { - destroy(); + // shared_from_this is not yet allows in destructor + // be sure to call destroy() first !!! + // destroy(); } void BeaconEmitter::lock() @@ -141,9 +143,7 @@ void BeaconEmitter::reschedule() void BeaconEmitter::callback() { - // requires this instance has already a valid pointer to this - TransportSender::shared_pointer thisSender = shared_from_this(); - _transport->enqueueSendRequest(thisSender); + _transport->enqueueSendRequest(shared_from_this()); } }} diff --git a/pvAccessApp/server/serverChannelImpl.cpp b/pvAccessApp/server/serverChannelImpl.cpp index 83a5c78..0de84ea 100644 --- a/pvAccessApp/server/serverChannelImpl.cpp +++ b/pvAccessApp/server/serverChannelImpl.cpp @@ -53,19 +53,20 @@ void ServerChannelImpl::registerRequest(const pvAccessID id, Destroyable::shared void ServerChannelImpl::unregisterRequest(const pvAccessID id) { Lock guard(_mutex); - _iter = _requests.find(id); - if(_iter != _requests.end()) + std::map::iterator iter = _requests.find(id); + if(iter != _requests.end()) { - _requests.erase(_iter); + _requests.erase(iter); } } Destroyable::shared_pointer ServerChannelImpl::getRequest(const pvAccessID id) { - _iter = _requests.find(id); - if(_iter != _requests.end()) + Lock guard(_mutex); + std::map::iterator iter = _requests.find(id); + if(iter != _requests.end()) { - return _iter->second; + return iter->second; } return Destroyable::shared_pointer(); } @@ -74,7 +75,6 @@ void ServerChannelImpl::destroy() { Lock guard(_mutex); if (_destroyed) return; - _destroyed = true; // destroy all requests @@ -111,8 +111,7 @@ void ServerChannelImpl::destroyAllRequests() while(_requests.size() != 0) { - _iter = _requests.begin(); - _iter->second->destroy(); + _requests.begin()->second->destroy(); } _requests.clear(); diff --git a/pvAccessApp/server/serverChannelImpl.h b/pvAccessApp/server/serverChannelImpl.h index 59c3c4f..b507f61 100644 --- a/pvAccessApp/server/serverChannelImpl.h +++ b/pvAccessApp/server/serverChannelImpl.h @@ -18,10 +18,8 @@ namespace pvAccess { class ServerChannelImpl : public ServerChannel { public: - typedef std::tr1::shared_ptr shared_pointer; - typedef std::tr1::shared_ptr const_shared_pointer; - typedef std::tr1::weak_ptr weak_pointer; - typedef std::tr1::weak_ptr const_weak_pointer; + POINTER_DEFINITIONS(ServerChannelImpl); + /** * Create server channel for given process variable. * @param channel local channel. @@ -117,12 +115,6 @@ private: */ std::map _requests; - /** - * Requests iterator. - */ - // TODO remove - std::map::iterator _iter; - /** * Destroy state. */ diff --git a/pvAccessApp/server/serverContext.cpp b/pvAccessApp/server/serverContext.cpp index 4294773..838f19f 100644 --- a/pvAccessApp/server/serverContext.cpp +++ b/pvAccessApp/server/serverContext.cpp @@ -69,23 +69,24 @@ void ServerContextImpl::initializeLogger() //createFileLogger("serverContextImpl.log"); } - struct noop_deleter - { - template void operator()(T * p) - { - } - }; +struct noop_deleter +{ + template void operator()(T * p) {} +}; Configuration::shared_pointer ServerContextImpl::getConfiguration() { - ConfigurationProvider* configurationProvider = ConfigurationFactory::getProvider(); - Configuration* config = configurationProvider->getConfiguration("pvAccess-server"); - if (config == NULL) + Lock guard(_mutex); + if (configuration.get() == 0) { - config = configurationProvider->getConfiguration("system"); + ConfigurationProvider::shared_pointer configurationProvider = ConfigurationFactory::getProvider(); + configuration = configurationProvider->getConfiguration("pvAccess-server"); + if (configuration.get() == 0) + { + configuration = configurationProvider->getConfiguration("system"); + } } - return Configuration::shared_pointer(config, noop_deleter()); - // TODO cache !!! + return configuration; } /** @@ -166,21 +167,20 @@ std::auto_ptr ServerContextImpl::createResponseHandler() void ServerContextImpl::internalInitialize() { - osiSockAttach(); - _timer.reset(new Timer("pvAccess-server timer",lowerPriority)); + osiSockAttach(); + + _timer.reset(new Timer("pvAccess-server timer", lowerPriority)); _transportRegistry.reset(new TransportRegistry()); // setup broadcast UDP transport initializeBroadcastTransport(); - Context::shared_pointer thisContext = shared_from_this(); - ResponseHandlerFactory::shared_pointer thisFactory = shared_from_this(); - _acceptor.reset(new BlockingTCPAcceptor(thisContext, thisFactory, _serverPort, _receiveBufferSize)); + ServerContextImpl::shared_pointer thisServerContext = shared_from_this(); + + _acceptor.reset(new BlockingTCPAcceptor(thisServerContext, thisServerContext, _serverPort, _receiveBufferSize)); _serverPort = ntohs(_acceptor->getBindAddress()->ia.sin_port); - ServerContextImpl::shared_pointer thisServerContext = shared_from_this(); - Transport::shared_pointer transport = _broadcastTransport; - _beaconEmitter.reset(new BeaconEmitter(transport, thisServerContext)); + _beaconEmitter.reset(new BeaconEmitter(_broadcastTransport, thisServerContext)); } void ServerContextImpl::initializeBroadcastTransport() @@ -318,7 +318,11 @@ void ServerContextImpl::destroy() Lock guard(_mutex); if (_state == DESTROYED) { - THROW_BASE_EXCEPTION("Context already destroyed."); + // silent return + return; + // exception is not OK, since we use + // shared_pointer-s auto-cleanup/destruction + // THROW_BASE_EXCEPTION("Context already destroyed."); } // shutdown if not already @@ -519,6 +523,7 @@ std::string ServerContextImpl::getChannelProviderName() return _channelProviderNames; } +// NOTE: not synced void ServerContextImpl::setChannelProviderName(std::string channelProviderName) { if (_state != NOT_INITIALIZED) diff --git a/pvAccessApp/server/serverContext.h b/pvAccessApp/server/serverContext.h index 6f79b63..7e5c849 100644 --- a/pvAccessApp/server/serverContext.h +++ b/pvAccessApp/server/serverContext.h @@ -402,6 +402,8 @@ private: * Destroy all transports. */ void destroyAllTransports(); + + Configuration::shared_pointer configuration; }; diff --git a/pvAccessApp/utils/configuration.cpp b/pvAccessApp/utils/configuration.cpp index e1f1707..0bf7ff6 100644 --- a/pvAccessApp/utils/configuration.cpp +++ b/pvAccessApp/utils/configuration.cpp @@ -16,47 +16,43 @@ using namespace std; Properties::Properties() { _fileName = ""; - _infile = new ifstream(); + _infile.reset(new ifstream()); _infile->exceptions (ifstream::failbit | ifstream::badbit ); - _outfile = new ofstream(); + _outfile.reset(new ofstream()); _outfile->exceptions (ofstream::failbit | ofstream::badbit ); } Properties::Properties(const string fileName) { _fileName = fileName; - _infile = new ifstream(); + _infile.reset(new ifstream()); _infile->exceptions (ifstream::failbit | ifstream::badbit ); - _outfile = new ofstream(); + _outfile.reset(new ofstream()); _outfile->exceptions (ofstream::failbit | ofstream::badbit ); } Properties::~Properties() { - delete _infile; - delete _outfile; - //clear map - _properties.clear(); } void Properties::setProperty(const string key,const string value) { string oldValue; - _propertiesIterator = _properties.find(key); + std::map::iterator propertiesIterator = _properties.find(key); - if(_propertiesIterator != _properties.end()) //found in map + if(propertiesIterator != _properties.end()) //found in map { - _properties.erase(_propertiesIterator); + _properties.erase(propertiesIterator); } _properties[key] = value; } string Properties::getProperty(const string key) { - _propertiesIterator = _properties.find(key); - if(_propertiesIterator != _properties.end()) //found in map + std::map::iterator propertiesIterator = _properties.find(key); + if(propertiesIterator != _properties.end()) //found in map { - return string(_propertiesIterator->second); + return string(propertiesIterator->second); } else { @@ -67,10 +63,10 @@ string Properties::getProperty(const string key) string Properties::getProperty(const string key, const string defaultValue) { - _propertiesIterator = _properties.find(key); - if(_propertiesIterator != _properties.end()) //found in map + std::map::iterator propertiesIterator = _properties.find(key); + if(propertiesIterator != _properties.end()) //found in map { - return string(_propertiesIterator->second); + return string(propertiesIterator->second); } _properties[key] = defaultValue; @@ -160,13 +156,13 @@ void Properties::store() } - for (_propertiesIterator = _properties.begin() ; - _propertiesIterator != _properties.end(); - _propertiesIterator++ ) + for (std::map::iterator propertiesIterator = _properties.begin(); + propertiesIterator != _properties.end(); + propertiesIterator++ ) { try { - string line = string(_propertiesIterator->first) + string("=") + string(_propertiesIterator->second) + string("\n"); + string line = string(propertiesIterator->first) + string("=") + string(propertiesIterator->second) + string("\n"); _outfile->write(line.c_str(),line.length()); } catch (ofstream::failure& e) { @@ -186,28 +182,27 @@ void Properties::store(const string fileName) void Properties::list() { - for (_propertiesIterator = _properties.begin() ; - _propertiesIterator != _properties.end(); - _propertiesIterator++ ) + for (std::map::iterator propertiesIterator = _properties.begin() ; + propertiesIterator != _properties.end(); + propertiesIterator++ ) { - cout << "Key:" << _propertiesIterator->first << ",Value: " << _propertiesIterator->second << endl; + cout << "Key:" << propertiesIterator->first << ",Value: " << propertiesIterator->second << endl; } } -SystemConfigurationImpl::SystemConfigurationImpl() +SystemConfigurationImpl::SystemConfigurationImpl() : + _properties(new Properties()) { _envParam.name = new char[256]; _envParam.pdflt = NULL; // no exception, default value is taken //_ibuffer.exceptions ( ifstream::failbit | ifstream::badbit ); //_obuffer.exceptions ( ifstream::failbit | ifstream::badbit ); - _properties = new Properties(); } SystemConfigurationImpl::~SystemConfigurationImpl() { if(_envParam.name) delete[] _envParam.name; - if(_properties) delete _properties; } bool SystemConfigurationImpl::getPropertyAsBoolean(const string name, const bool defaultValue) @@ -288,18 +283,13 @@ ConfigurationProviderImpl::ConfigurationProviderImpl() ConfigurationProviderImpl::~ConfigurationProviderImpl() { - for(_configsIter = _configs.begin(); _configsIter != _configs.end(); _configsIter++) - { - delete _configsIter->second; - } - _configs.clear(); } -void ConfigurationProviderImpl::registerConfiguration(const string name, const Configuration* configuration) +void ConfigurationProviderImpl::registerConfiguration(const string name, Configuration::shared_pointer const & configuration) { Lock guard(_mutex); - _configsIter = _configs.find(name); - if(_configsIter != _configs.end()) + std::map::iterator configsIter = _configs.find(name); + if(configsIter != _configs.end()) { string msg = "configuration with name " + name + " already registered"; THROW_BASE_EXCEPTION(msg.c_str()); @@ -307,29 +297,30 @@ void ConfigurationProviderImpl::registerConfiguration(const string name, const C _configs[name] = configuration; } -Configuration* ConfigurationProviderImpl::getConfiguration(const string name) +Configuration::shared_pointer ConfigurationProviderImpl::getConfiguration(const string name) { - _configsIter = _configs.find(name); - if(_configsIter != _configs.end()) + std::map::iterator configsIter = _configs.find(name); + if(configsIter != _configs.end()) { - return const_cast(_configsIter->second); + return configsIter->second; } - return NULL; + return Configuration::shared_pointer(); } -ConfigurationProviderImpl* ConfigurationFactory::_configurationProvider = NULL; -Mutex ConfigurationFactory::_conf_factory_mutex; +ConfigurationProvider::shared_pointer configurationProvider; +Mutex conf_factory_mutex; -ConfigurationProviderImpl* ConfigurationFactory::getProvider() +ConfigurationProvider::shared_pointer ConfigurationFactory::getProvider() { - Lock guard(_conf_factory_mutex); - if(_configurationProvider == NULL) + Lock guard(conf_factory_mutex); + if(configurationProvider.get() == NULL) { - _configurationProvider = new ConfigurationProviderImpl(); + configurationProvider.reset(new ConfigurationProviderImpl()); // default - _configurationProvider->registerConfiguration("system", new SystemConfigurationImpl()); + Configuration::shared_pointer systemConfig(new SystemConfigurationImpl()); + configurationProvider->registerConfiguration("system", systemConfig); } - return _configurationProvider; + return configurationProvider; } }} diff --git a/pvAccessApp/utils/configuration.h b/pvAccessApp/utils/configuration.h index 0977250..643b6fe 100644 --- a/pvAccessApp/utils/configuration.h +++ b/pvAccessApp/utils/configuration.h @@ -21,8 +21,6 @@ #include #include -// TODO implement using smart pointers - namespace epics { namespace pvAccess { @@ -45,9 +43,8 @@ public: private: std::map _properties; - std::map::iterator _propertiesIterator; - std::ifstream* _infile; - std::ofstream* _outfile; + std::auto_ptr _infile; + std::auto_ptr _outfile; std::string _fileName; inline void truncate(std::string& str) @@ -139,7 +136,7 @@ public: float getPropertyAsFloat(const std::string name, const float defaultValue); float getPropertyAsDouble(const std::string name, const double defaultValue); std::string getPropertyAsString(const std::string name, std::string defaultValue); - Properties* _properties; + std::auto_ptr _properties; private: ENV_PARAM _envParam; std::istringstream _ibuffer; @@ -153,6 +150,7 @@ private: class ConfigurationProvider : private epics::pvData::NoDefaultMethods { public: + POINTER_DEFINITIONS(ConfigurationProvider); /** * Destructor. */ @@ -164,14 +162,14 @@ public: * * @return configuration specified by name or NULL if it does not exists. */ - virtual Configuration* getConfiguration(const std::string name) = 0; + virtual Configuration::shared_pointer getConfiguration(const std::string name) = 0; /** * Register configuration. * * @param name name of the configuration to register. * @param configuration configuration to register. */ - virtual void registerConfiguration(const std::string name, const Configuration* configuration) = 0; + virtual void registerConfiguration(const std::string name, Configuration::shared_pointer const & configuration) = 0; }; class ConfigurationProviderImpl: public ConfigurationProvider @@ -182,12 +180,11 @@ public: * Destructor. Note: Registered configurations will be deleted!! */ ~ConfigurationProviderImpl(); - Configuration* getConfiguration(const std::string name); - void registerConfiguration(const std::string name, const Configuration* configuration); + Configuration::shared_pointer getConfiguration(const std::string name); + void registerConfiguration(const std::string name, Configuration::shared_pointer const & configuration); private: epics::pvData::Mutex _mutex; - std::map _configs; - std::map::iterator _configsIter; + std::map _configs; }; /** @@ -196,6 +193,8 @@ private: class ConfigurationFactory : private epics::pvData::NoDefaultMethods { public: + POINTER_DEFINITIONS(ConfigurationFactory); + /** * Lazily creates configuration provider. * @@ -204,12 +203,10 @@ public: * * @return configuration provider */ - static ConfigurationProviderImpl* getProvider(); + static ConfigurationProvider::shared_pointer getProvider(); private: ConfigurationFactory() {}; - static ConfigurationProviderImpl* _configurationProvider; - static epics::pvData::Mutex _conf_factory_mutex; }; }}