From c56091978c8d91c95742d49d1315acc478748e3f Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Thu, 23 Jul 2015 17:57:30 -0500 Subject: [PATCH] db: Fixed obscure dbCa bug This could be triggered if dbCaRemoveLink() is called on a link for which there is an outstanding dbCaPutCallback(). Normally a dbCaPutCallback() callback routine is passed the associated userPvt pointer as an argument, but in the event that dbCaRemoveLink() gets used on the same link between the put and its completion callback, the callback routine was being called with the link pointer as the argument instead. For all the existing Asyn Soft Channel device supports this is not a problem as they all pass the link pointer as their userPvt argument, but that won't necessarily always be the case. Also updated the comments describing the process of removing links. --- src/db/dbCa.c | 36 +++++++++++++++--------------------- src/db/dbCaPvt.h | 1 - 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/src/db/dbCa.c b/src/db/dbCa.c index bb976bd5b..27ed3cb8b 100644 --- a/src/db/dbCa.c +++ b/src/db/dbCa.c @@ -98,7 +98,7 @@ static void dbCaTask(void *); * that has been deleted. * * Just a few words about handling dbCaRemoveLink because this is when - * it is essential that nothing trys to use a caLink that has been freed. + * it is essential that nothing tries to use a caLink that has been freed. * * dbCaRemoveLink is called when links are being modified. This is only * done with the dbScan mechanism guranteeing that nothing from @@ -117,19 +117,16 @@ static void dbCaTask(void *); * dbCaTask issues a ca_clear_channel and then frees the caLink. * * If any channel access callback gets called before the ca_clear_channel - * it finds pca->plink=0 and does nothing. Once ca_clear_channel + * it finds pca->plink==0 and does nothing. Once ca_clear_channel * is called no other callback for this caLink will be called. * * dbCaPutLinkCallback causes an additional complication because * when dbCaRemoveLink is called the callback may not have occured. - * What is done is the following: - * If callback has not occured dbCaRemoveLink sets plinkPutCallback=plink - * If putCallback is called before dbCaTask calls ca_clear_channel - * it does NOT call the users callback. - * dbCaTask calls the users callback passing plinkPutCallback AFTER - * it has called ca_clear_channel - * Thus the users callback will get called exactly once. -*/ + * If putComplete sees plink==0 it will not call the user's code. + * When dbCaTask performs a CA_CLEAR_CHANNEL action, if pca->putCallback + * is non-zero it will call that callback AFTER ca_clear_channel. + * Thus the user's callback will get called exactly once. + */ static void addAction(caLink *pca, short link_action) { @@ -163,9 +160,9 @@ static void addAction(caLink *pca, short link_action) epicsEventSignal(workListEvent); } -void dbCaCallbackProcess(void *usrPvt) +void dbCaCallbackProcess(void *userPvt) { - struct link *plink = (struct link *)usrPvt; + struct link *plink = (struct link *)userPvt; dbCommon *pdbCommon = plink->value.pv_link.precord; dbScanLock(pdbCommon); @@ -239,8 +236,6 @@ void dbCaRemoveLink(struct link *plink) epicsMutexMustLock(pca->lock); pca->plink = 0; plink->value.pv_link.pvt = 0; - if (pca->putCallback) - pca->plinkPutCallback = plink; /* Unlock before addAction or dbCaTask might free first */ epicsMutexUnlock(pca->lock); addAction(pca, CA_CLEAR_CHANNEL); @@ -735,7 +730,7 @@ static void exceptionCallback(struct exception_handler_args args) } } -static void putCallback(struct event_handler_args arg) +static void putComplete(struct event_handler_args arg) { caLink *pca = (caLink *)arg.usr; struct link *plink; @@ -866,7 +861,7 @@ static void dbCaTask(void *arg) epicsMutexUnlock(workListLock); /* Give back immediately */ if (link_action & CA_CLEAR_CHANNEL) { /* This must be first */ dbCaCallback callback; - struct link *plinkPutCallback = 0; + void *userPvt = 0; if (pca->chid) { ca_clear_channel(pca->chid); @@ -874,8 +869,7 @@ static void dbCaTask(void *arg) } callback = pca->putCallback; if (callback) { - plinkPutCallback = pca->plinkPutCallback; - pca->plinkPutCallback = 0; + userPvt = pca->putUserPvt; pca->putCallback = 0; pca->putType = 0; } @@ -887,7 +881,7 @@ static void dbCaTask(void *arg) epicsMutexDestroy(pca->lock); free(pca); /* No alarm is raised. Since link is changing so what? */ - if (callback) callback(plinkPutCallback); + if (callback) callback(userPvt); continue; /* No other link_action makes sense */ } if (link_action & CA_CONNECT) { @@ -921,7 +915,7 @@ static void dbCaTask(void *arg) status = ca_array_put_callback( pca->dbrType, pca->nelements, pca->chid, pca->pputNative, - putCallback, pca); + putComplete, pca); } else { status = ECA_PUTFAIL; } @@ -944,7 +938,7 @@ static void dbCaTask(void *arg) status = ca_array_put_callback( DBR_STRING, 1, pca->chid, pca->pputString, - putCallback, pca); + putComplete, pca); } else { status = ECA_PUTFAIL; } diff --git a/src/db/dbCaPvt.h b/src/db/dbCaPvt.h index 9625bfbd6..a0a1f65a0 100644 --- a/src/db/dbCaPvt.h +++ b/src/db/dbCaPvt.h @@ -58,7 +58,6 @@ typedef struct caLink short putType; dbCaCallback putCallback; void *putUserPvt; - struct link *plinkPutCallback; /* The following are for access to additional attributes*/ char gotAttributes; dbCaCallback getAttributes;