epics/pvAccess
0
0
Fork 0
Code Issues Pull Requests Projects Wiki Activity

changes explained in MRKCHANGES.md

Browse Source
This commit is contained in:
mrkraimer
2017-06-04 07:49:28 -04:00
parent 65cb1c8397
commit 0f8a3b2b7d
7 changed files with 281 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files

103
MRKCHANGES.md Normal file
View File

@@ -0,0 +1,103 @@
========
Overview
========
pvaCientCPP did not terminate properly.
It has a singleton class named **PvaClient**.
When the singleton is created it calls:
ClientFactory::start();
The destructor for **PvaClient** calls:
ClientFactory::stop();
This call results in the following message:
terminate called after throwing an instance of 'epicsMutex::invalidMutex'
what(): epicsMutex::invalidMutex()
Further investigation found that this was caused by a mutex belonging to ChannelProviderRegistry.
If fact it appeared that the only way the crash did not occur is if the client code was of the form:
int main(int argc,char *argv[])
{
ClientFactory::start();
...
ClientFactory::stop();
}
While investigating this problem I also realized that the existing implementation of
ClientFactory only worked if a single call was made to start and a single call to stop.
In pvAccessCPP changes were made to the followimg:
1) ChannelProviderRegistry
2) ClientFactory
3) CAChannelProviderFactory
These are discussed below.
Changes were also made to pvaClientCPP.
The change was to use the new ChannelProviderRegistry methods.
All these changes are in the github repositories belonging to mrkraimer.
-----------------------
ChannelProviderRegistry
-----------------------
The following new methods are added to ChannelProviderRegistry:
static ChannelProviderRegistry::shared_pointer getChannelProviderRegistry();
void registerChannelProviderFactory(
ChannelProviderFactory::shared_pointer const & channelProviderFactory);
void unregisterChannelProviderFactory(
ChannelProviderFactory::shared_pointer const & channelProviderFactory);
**getChannelProviderRegistry** creates the single instance of **ChannelProviderRegistry**
the first time it is called and always returns a shared pointer to the single
instance,
Any facility that calls this must keep the shared pointer until the faciliity no longer requires
use of the registry or any channel provider the facliity uses.
The above methods replace the following functions:
// THE FOLLOWING SHOULD NOT BE CALLED
epicsShareFunc ChannelProviderRegistry::shared_pointer getChannelProviderRegistry() EPICS_DEPRECATED;
epicsShareFunc void registerChannelProviderFactory(
ChannelProviderFactory::shared_pointer const & channelProviderFactory) EPICS_DEPRECATED;
epicsShareFunc void unregisterChannelProviderFactory(
ChannelProviderFactory::shared_pointer const & channelProviderFactory) EPICS_DEPRECATED;
epicsShareFunc void unregisterAllChannelProviderFactory() EPICS_DEPRECATED;
Note that the methods are all deprecated.
They were kept so that existing code will still compile and run.
Any code that uses these methods may see the invalidMutex exception on termination.
-------------
ClientFactory
-------------
This has two major changes:
1) It uses the new **ChannelProviderRegistry** methods,
2) It allows multiple calls to start and stop.
------------------------
CAChannelProviderFactory
------------------------
This has two major changes:
1) It uses the new **ChannelProviderRegistry** methods,
2) It allows multiple calls to start and stop.

89
src/ca/caProvider.cpp
View File

@@ -15,9 +15,12 @@
#include <pv/caProvider.h>
#include <pv/caChannel.h>
namespace epics {
namespace pvAccess {
namespace ca {
using namespace epics::pvData;
using namespace epics::pvAccess;
using namespace epics::pvAccess::ca;
#define EXCEPTION_GUARD(code) try { code; } \
catch (std::exception &e) { LOG(logLevelError, "Unhandled exception caught from client code at %s:%d: %s", __FILE__, __LINE__, e.what()); } \
@@ -165,23 +168,25 @@ void CAChannelProvider::initialize()
}
// TODO global static variable (de/initialization order not guaranteed)
static Mutex mutex;
static CAChannelProvider::shared_pointer sharedProvider;
class CAChannelProviderFactoryImpl : public ChannelProviderFactory
class CAChannelProviderFactory : public ChannelProviderFactory
{
private:
Mutex m_mutex;
CAChannelProvider::shared_pointer sharedProvider;
public:
POINTER_DEFINITIONS(CAChannelProviderFactoryImpl);
POINTER_DEFINITIONS(CAChannelProviderFactory);
virtual ~CAChannelProviderFactory()
{
Lock guard(m_mutex);
if (sharedProvider)
{
CAChannelProvider::shared_pointer provider;
sharedProvider.swap(provider);
// factroy cleans up also shared provider
provider->destroy();
}
}
virtual std::string getFactoryName()
{
@@ -190,8 +195,8 @@ public:
virtual ChannelProvider::shared_pointer sharedInstance()
{
Lock guard(mutex);
if (!sharedProvider.get())
Lock guard(m_mutex);
if (!sharedProvider)
{
try {
// TODO use std::make_shared
@@ -224,35 +229,47 @@ public:
}
}
void destroySharedInstance()
{
if(!sharedProvider) return;
sharedProvider->destroy();
sharedProvider.reset();
}
};
static CAChannelProviderFactoryImpl::shared_pointer factory;
static Mutex startStopMutex;
ChannelProviderRegistry::shared_pointer CAClientFactory::channelRegistry = ChannelProviderRegistry::shared_pointer();
CAChannelProviderFactoryPtr CAClientFactory::channelProvider = CAChannelProviderFactory::shared_pointer();
int CAClientFactory::numStart = 0;
void CAClientFactory::start()
{
Lock guard(startStopMutex);
std::cout << "CAClientFactory::start() numStart " << numStart << std::endl;
++numStart;
if(numStart>1) return;
epicsSignalInstallSigAlarmIgnore();
epicsSignalInstallSigPipeIgnore();
Lock guard(mutex);
if (!factory.get())
factory.reset(new CAChannelProviderFactoryImpl());
registerChannelProviderFactory(factory);
channelProvider.reset(new CAChannelProviderFactory());
channelRegistry = ChannelProviderRegistry::getChannelProviderRegistry();
std::cout << "channelRegistry::use_count " << channelRegistry.use_count() << std::endl;
channelRegistry->registerChannelProviderFactory(channelProvider);
}
void CAClientFactory::stop()
{
Lock guard(mutex);
if (factory.get())
std::cout << "ClientFactory::stop() numStart " << numStart << std::endl;
std::cout << "channelRegistry::use_count " << channelRegistry.use_count() << std::endl;
Lock guard(startStopMutex);
if(numStart==0) return;
--numStart;
if(numStart>=1) return;
if (channelProvider)
{
unregisterChannelProviderFactory(factory);
factory->destroySharedInstance();
channelRegistry->unregisterChannelProviderFactory(channelProvider);
if(!channelProvider.unique()) {
LOG(logLevelWarn, "ClientFactory::stop() finds shared client context with %u remaining users",
(unsigned)channelProvider.use_count());
}
channelProvider.reset();
channelRegistry.reset();
}
}
}}}

11
src/ca/pv/caProvider.h
View File

@@ -19,7 +19,7 @@ namespace pvAccess {
namespace ca {
class epicsShareClass CAChannelProvider :
public ChannelProvider,
public epics::pvAccess::ChannelProvider,
public std::tr1::enable_shared_from_this<CAChannelProvider>
{
public:
@@ -82,9 +82,14 @@ private:
bool destroyed;
};
class CAChannelProviderFactory;
typedef std::tr1::shared_ptr<CAChannelProviderFactory> CAChannelProviderFactoryPtr;
class epicsShareClass CAClientFactory
{
class epicsShareClass CAClientFactory {
private:
static epics::pvAccess::ChannelProviderRegistry::shared_pointer channelRegistry;
static CAChannelProviderFactoryPtr channelProvider;
static int numStart;
public:
static void start();
static void stop();

38
src/client/pv/pvAccess.h
View File

@@ -998,8 +998,30 @@ class epicsShareClass ChannelProviderRegistry {
public:
POINTER_DEFINITIONS(ChannelProviderRegistry);
~ChannelProviderRegistry();
typedef std::vector<std::string> stringVector_t;
/**
* Get a shared instance of the single instance of ChannelProviderRegistry.
* The caller must keep the shared instance until the caller no longer needs
* the registry. The first call creates the single instance.
* @return The interface for ChannelProviderRegistry
*/
static ChannelProviderRegistry::shared_pointer getChannelProviderRegistry();
/**
* Register a ChannelProviderFactory.
* @param channelProviderFactory The ChannelProviderFactory.
*/
void registerChannelProviderFactory(
ChannelProviderFactory::shared_pointer const & channelProviderFactory);
/**
* Unregister a ChannelProviderFactory.
* @param channelProviderFactory The ChannelProviderFactory.
*/
void unregisterChannelProviderFactory(
ChannelProviderFactory::shared_pointer const & channelProviderFactory);
/**
* Get a shared instance of the provider with the specified name.
* @param providerName The name of the provider.
@@ -1064,18 +1086,22 @@ public:
}
private:
ChannelProviderRegistry() {}
// ChannelProviderRegistry() {}
ChannelProviderRegistry();
epics::pvData::Mutex mutex;
typedef std::map<std::string, ChannelProviderFactory::shared_pointer> providers_t;
providers_t providers;
};
//! never returns NULL
epicsShareFunc ChannelProviderRegistry::shared_pointer getChannelProviderRegistry();
epicsShareFunc void registerChannelProviderFactory(ChannelProviderFactory::shared_pointer const & channelProviderFactory);
epicsShareFunc void unregisterChannelProviderFactory(ChannelProviderFactory::shared_pointer const & channelProviderFactory);
epicsShareFunc void unregisterAllChannelProviderFactory();
// THE FOLLOWING SHOULD NOT BE CALLED
epicsShareFunc ChannelProviderRegistry::shared_pointer getChannelProviderRegistry() EPICS_DEPRECATED;
epicsShareFunc void registerChannelProviderFactory(
ChannelProviderFactory::shared_pointer const & channelProviderFactory) EPICS_DEPRECATED;
epicsShareFunc void unregisterChannelProviderFactory(
ChannelProviderFactory::shared_pointer const & channelProviderFactory) EPICS_DEPRECATED;
epicsShareFunc void unregisterAllChannelProviderFactory() EPICS_DEPRECATED;
/**

55
src/factory/ChannelAccessFactory.cpp
View File

@@ -21,6 +21,42 @@ using std::string;
namespace epics {
namespace pvAccess {
ChannelProviderRegistry::ChannelProviderRegistry()
{
std::cout << "ChannelProviderRegistry\n";
}
ChannelProviderRegistry::~ChannelProviderRegistry()
{
std::cout << "~ChannelProviderRegistry\n";
}
ChannelProviderRegistry::shared_pointer ChannelProviderRegistry::getChannelProviderRegistry()
{
static Mutex mutex;
static ChannelProviderRegistry::shared_pointer global_reg;
Lock guard(mutex);
if(!global_reg) {
global_reg = ChannelProviderRegistry::build();
}
return global_reg;
}
void ChannelProviderRegistry::registerChannelProviderFactory(
ChannelProviderFactory::shared_pointer const & channelProviderFactory)
{
assert(channelProviderFactory);
add(channelProviderFactory);
}
void ChannelProviderRegistry::unregisterChannelProviderFactory(
ChannelProviderFactory::shared_pointer const & channelProviderFactory)
{
assert(channelProviderFactory);
remove(channelProviderFactory->getFactoryName());
}
ChannelProvider::shared_pointer ChannelProviderRegistry::getProvider(std::string const & providerName) {
ChannelProviderFactory::shared_pointer fact(getFactory(providerName));
if(fact)
@@ -78,6 +114,7 @@ bool ChannelProviderRegistry::add(const ChannelProviderFactory::shared_pointer&
}
ChannelProviderFactory::shared_pointer ChannelProviderRegistry::remove(const std::string& name)
{
ChannelProviderFactory::shared_pointer ret;
providers_t::iterator iter(providers.find(name));
@@ -89,24 +126,18 @@ ChannelProviderFactory::shared_pointer ChannelProviderRegistry::remove(const std
}
ChannelProviderRegistry::shared_pointer getChannelProviderRegistry() {
static Mutex mutex;
static ChannelProviderRegistry::shared_pointer global_reg;
Lock guard(mutex);
if(!global_reg) {
global_reg = ChannelProviderRegistry::build();
}
return global_reg;
std::cerr << "getChannelProviderRegistry should not be used\n";
return ChannelProviderRegistry::getChannelProviderRegistry();
}
void registerChannelProviderFactory(ChannelProviderFactory::shared_pointer const & channelProviderFactory) {
assert(channelProviderFactory);
getChannelProviderRegistry()->add(channelProviderFactory);
std::cerr << "registerChannelProviderFactory should not be used\n";
getChannelProviderRegistry()->registerChannelProviderFactory(channelProviderFactory);
}
void unregisterChannelProviderFactory(ChannelProviderFactory::shared_pointer const & channelProviderFactory) {
assert(channelProviderFactory);
getChannelProviderRegistry()->remove(channelProviderFactory->getFactoryName());
std::cerr << "unregisterChannelProviderFactory should not be used\n";
getChannelProviderRegistry()->unregisterChannelProviderFactory(channelProviderFactory);
}
epicsShareFunc void unregisterAllChannelProviderFactory()

45
src/pva/clientFactory.cpp
View File

@@ -15,8 +15,10 @@
#include <pv/clientFactory.h>
#include <pv/clientContextImpl.h>
namespace epics {
namespace pvAccess {
using namespace epics::pvData;
using namespace epics::pvAccess;
class ChannelProviderFactoryImpl : public ChannelProviderFactory
{
@@ -62,32 +64,45 @@ public:
}
};
static Mutex cprovfact_mutex;
static ChannelProviderFactoryImpl::shared_pointer pva_factory;
static Mutex startStopMutex;
ChannelProviderRegistryPtr ClientFactory::channelRegistry = ChannelProviderRegistryPtr();
ChannelProviderFactoryImplPtr ClientFactory::channelProvider = ChannelProviderFactoryImplPtr();
int ClientFactory::numStart = 0;
void ClientFactory::start()
{
Lock guard(startStopMutex);
std::cout << "ClientFactory::start() numStart " << numStart << std::endl;
++numStart;
if(numStart>1) return;
epicsSignalInstallSigAlarmIgnore();
epicsSignalInstallSigPipeIgnore();
Lock guard(cprovfact_mutex);
if (!pva_factory)
pva_factory.reset(new ChannelProviderFactoryImpl());
registerChannelProviderFactory(pva_factory);
channelProvider.reset(new ChannelProviderFactoryImpl());
channelRegistry = ChannelProviderRegistry::getChannelProviderRegistry();
std::cout << "channelRegistry::use_count " << channelRegistry.use_count() << std::endl;
channelRegistry->registerChannelProviderFactory(channelProvider);
}
void ClientFactory::stop()
{
Lock guard(cprovfact_mutex);
std::cout << "ClientFactory::stop() numStart " << numStart << std::endl;
std::cout << "channelRegistry::use_count " << channelRegistry.use_count() << std::endl;
Lock guard(startStopMutex);
if(numStart==0) return;
--numStart;
if(numStart>=1) return;
if (pva_factory)
if (channelProvider)
{
unregisterChannelProviderFactory(pva_factory);
if(!pva_factory.unique()) {
channelRegistry->unregisterChannelProviderFactory(channelProvider);
if(!channelProvider.unique()) {
LOG(logLevelWarn, "ClientFactory::stop() finds shared client context with %u remaining users",
(unsigned)pva_factory.use_count());
(unsigned)channelProvider.use_count());
}
pva_factory.reset();
channelProvider.reset();
channelRegistry.reset();
}
}
}}

12
src/pva/pv/clientFactory.h
View File

@@ -9,10 +9,22 @@
#include <shareLib.h>
#include <pv/sharedPtr.h>
namespace epics {
namespace pvAccess {
class ChannelProviderRegistry;
typedef std::tr1::shared_ptr<ChannelProviderRegistry> ChannelProviderRegistryPtr;
class ChannelProviderFactoryImpl;
typedef std::tr1::shared_ptr<ChannelProviderFactoryImpl> ChannelProviderFactoryImplPtr;
class epicsShareClass ClientFactory {
private:
static ChannelProviderRegistryPtr channelRegistry;
static ChannelProviderFactoryImplPtr channelProvider;
static int numStart;
public:
static void start();
static void stop();