Menu
▾
▴
[Sblim-commit] sfcb providerDrv.c, 1.108, 1.109 providerMgr.c, 1.79, 1.80 ChangeLog, 1.730, 1.731 NEWS, 1.653, 1.654
|
From: Chris B. <buc...@us...> - 2012-03-07 00:52:38
|
Update of /cvsroot/sblim/sfcb
In directory vz-cvs-3.sog:/tmp/cvs-serv21832
Modified Files:
providerDrv.c providerMgr.c ChangeLog NEWS
Log Message:
[ 3497950 ] SFCB Semaphore Handling Improvements
Index: NEWS
===================================================================
RCS file: /cvsroot/sblim/sfcb/NEWS,v
retrieving revision 1.653
retrieving revision 1.654
diff -u -d -r1.653 -r1.654
--- NEWS 6 Mar 2012 17:33:55 -0000 1.653
+++ NEWS 7 Mar 2012 00:52:36 -0000 1.654
@@ -44,6 +44,7 @@
- 3416060 sigsegv in tool_mm_flush()
- 3497096 double free during stopProc
- 3497765 Better Error Handling in sfcb Provider Manager and Driver
+- 3497950 SFCB Semaphore Handling Improvements
Changes in 1.3.13
=================
Index: providerMgr.c
===================================================================
RCS file: /cvsroot/sblim/sfcb/providerMgr.c,v
retrieving revision 1.79
retrieving revision 1.80
diff -u -d -r1.79 -r1.80
--- providerMgr.c 6 Mar 2012 17:33:55 -0000 1.79
+++ providerMgr.c 7 Mar 2012 00:52:36 -0000 1.80
@@ -1004,13 +1004,25 @@
int i = 0;
_SFCB_ENTER(TRACE_PROVIDERMGR, "closeProviderContext");
for(i = 0; i < ctx->pCount; i++) {
- semAcquire(sfcbSem,PROV_GUARD(ctx->pAs[i].ids.procId));
- if(semGetValue(sfcbSem, PROV_INUSE(ctx->pAs[i].ids.procId)) != 0) {
- semAcquire(sfcbSem,PROV_INUSE(ctx->pAs[i].ids.procId));
+ if (semAcquireUnDo(sfcbSem,PROV_GUARD(ctx->pAs[i].ids.procId))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error acquiring semaphore for %d, reason: %s\n",
+ ctx->pAs[i].ids.procId, strerror(errno));
+ _SFCB_ABORT();
+ }
+ if(semGetValue(sfcbSem, PROV_INUSE(ctx->pAs[i].ids.procId)) > 0) {
+ if (semAcquireUnDo(sfcbSem,PROV_INUSE(ctx->pAs[i].ids.procId))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error decreasing inuse semaphore for %d, reason: %s\n",
+ ctx->pAs[i].ids.procId, strerror(errno));
+ _SFCB_ABORT();
+ }
} else {
fprintf(stderr, "--- closeProviderContext not touching sem %d; already zero\n", PROV_INUSE(ctx->pAs[i].ids.procId));
}
- semRelease(sfcbSem,PROV_GUARD(ctx->pAs[i].ids.procId));
+ if (semReleaseUnDo(sfcbSem,PROV_GUARD(ctx->pAs[i].ids.procId))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error releasing semaphore for %d, reason: %s\n",
+ ctx->pAs[i].ids.procId, strerror(errno));
+ _SFCB_ABORT();
+ }
}
if (ctx->pAs) free(ctx->pAs);
}
@@ -1023,15 +1035,30 @@
_SFCB_ENTER(TRACE_PROVIDERMGR, "setInuseSem");
if (sfcbSem < 0) { //Semaphore Not initialized.
semKey=ftok(SFCB_BINARY,'S');
- sfcbSem=semget(semKey,1, 0600);
+ if ((sfcbSem=semget(semKey,1, 0600)) < 0) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error getting semaphore set, reason: %s\n",
+ strerror(errno));
+ _SFCB_ABORT();
+ }
}
ids.ids=id;
- semAcquire(sfcbSem,PROV_GUARD(ids.procId));
- //semAcquire(sfcbSem,PROV_INUSE(ids.procId));
- //semReleaseUnDo(sfcbSem,PROV_INUSE(ids.procId));
- semRelease(sfcbSem,PROV_GUARD(ids.procId));
+ if (semAcquireUnDo(sfcbSem,PROV_GUARD(ids.procId))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error acquiring semaphore for %d, reason: %s\n",
+ ids.procId, strerror(errno));
+ _SFCB_ABORT();
+ }
+ if (semReleaseUnDo(sfcbSem,PROV_INUSE(ids.procId))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error increasing inuse semaphore for %d, reason: %s\n",
+ ids.procId, strerror(errno));
+ _SFCB_ABORT();
+ }
+ if (semReleaseUnDo(sfcbSem,PROV_GUARD(ids.procId))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error releasing semaphore for %d, reason: %s\n",
+ ids.procId, strerror(errno));
+ _SFCB_ABORT();
+ }
_SFCB_EXIT();
}
Index: providerDrv.c
===================================================================
RCS file: /cvsroot/sblim/sfcb/providerDrv.c,v
retrieving revision 1.108
retrieving revision 1.109
diff -u -d -r1.108 -r1.109
--- providerDrv.c 6 Mar 2012 17:33:55 -0000 1.108
+++ providerDrv.c 7 Mar 2012 00:52:36 -0000 1.109
@@ -156,6 +156,9 @@
ProviderInfo *activProvs = NULL;
+static void increaseInUseSem(int id);
+static void decreaseInUseSem(int id);
+
unsigned long provSampleInterval=10;
unsigned long provTimeoutInterval=25;
unsigned provAutoGroup=0;
@@ -290,6 +293,52 @@
return count;
}
+static void increaseInUseSem(int id)
+{
+ _SFCB_ENTER(TRACE_PROVIDERDRV, "increaseInUseSem");
+
+ if (semAcquireUnDo(sfcbSem,PROV_GUARD(id))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error acquiring semaphore for %d, reason: %s\n",
+ id, strerror(errno));
+ _SFCB_ABORT();
+ }
+ if (semReleaseUnDo(sfcbSem,PROV_INUSE(id))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error increasing inuse semaphore for %d, reason: %s\n",
+ id, strerror(errno));
+ _SFCB_ABORT();
+ }
+ if (semReleaseUnDo(sfcbSem,PROV_GUARD(id))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error releasing semaphore for %d, reason: %s\n",
+ id, strerror(errno));
+ _SFCB_ABORT();
+ }
+ _SFCB_EXIT();
+}
+
+static void decreaseInUseSem(int id)
+{
+ _SFCB_ENTER(TRACE_PROVIDERDRV, "decreaseInUseSem");
+
+ if (semAcquireUnDo(sfcbSem,PROV_GUARD(id))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error acquiring semaphore for %d, reason: %s\n",
+ id, strerror(errno));
+ _SFCB_ABORT();
+ }
+ if (semGetValue(sfcbSem,PROV_INUSE(id)) > 0) {
+ if (semAcquireUnDo(sfcbSem,PROV_INUSE(id))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error decreasing inuse semaphore for %d, reason: %s\n",
+ id, strerror(errno));
+ _SFCB_ABORT();
+ }
+ }
+ if (semReleaseUnDo(sfcbSem,PROV_GUARD(id))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error releasing semaphore for %d, reason: %s\n",
+ id, strerror(errno));
+ _SFCB_ABORT();
+ }
+ _SFCB_EXIT();
+}
+
typedef struct _provLibAndTypes {
void* lib;
#define INST 1
@@ -474,8 +523,12 @@
if (pInfo) {
proc=curProvProc;
if (proc) {
- semAcquireUnDo(sfcbSem,PROV_GUARD(proc->id));
- if ((val=semGetValue(sfcbSem,PROV_INUSE(proc->id)))==0) {
+ if (semAcquireUnDo(sfcbSem,PROV_GUARD(proc->id))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error acquiring semaphore for %d, reason: %s\n",
+ proc->id, strerror(errno));
+ _SFCB_ABORT();
+ }
+ if ((val=semGetValue(sfcbSem,PROV_INUSE(proc->id)))==0) {
if ((now-proc->lastActivity)>provTimeoutInterval) {
ctx = native_new_CMPIContext(MEM_TRACKED,NULL);
noBreak=0;
@@ -542,7 +595,11 @@
}
}
}
- semRelease(sfcbSem,PROV_GUARD(proc->id));
+ if (semReleaseUnDo(sfcbSem,PROV_GUARD(proc->id))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error releasing semaphore for %d, reason: %s\n",
+ proc->id, strerror(errno));
+ _SFCB_ABORT();
+ }
}
}
}
@@ -692,19 +749,35 @@
for (i = 0; i < provProcMax; i++) {
if ((provProc+i) && provProc[i].pid &&
provProc[i].group && strcmp(provProc[i].group,info->group)==0) {
- semAcquire(sfcbSem,PROV_GUARD(provProc[i].id));
- semRelease(sfcbSem,PROV_INUSE(provProc[i].id));
- semRelease(sfcbSem,PROV_GUARD(provProc[i].id));
- info->pid=provProc[i].pid;
- info->providerSockets=provProc[i].providerSockets;
- _SFCB_TRACE(1,("--- Process %d shared by %s and %s",provProc[i].pid,info->providerName,
- provProc[i].firstProv->providerName));
- if (provProc[i].firstProv) info->next=provProc[i].firstProv;
- else info->next = NULL;
- provProc[i].firstProv=info;
- info->proc=provProc+i;
- if (info->unload<provProc[i].unload) provProc[i].unload=info->unload;
- _SFCB_RETURN(provProc[i].pid);
+ if (semAcquireUnDo(sfcbSem,PROV_GUARD(provProc[i].id))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error acquiring semaphore for %d, reason: %s\n",
+ provProc[i].id, strerror(errno));
+ _SFCB_ABORT();
+ }
+ /* double checking pattern required to prevent race ! */
+ if ((provProc+i) && provProc[i].pid &&
+ provProc[i].group && strcmp(provProc[i].group,info->group)==0) {
+ info->pid=provProc[i].pid;
+ info->providerSockets=provProc[i].providerSockets;
+ _SFCB_TRACE(1,("--- Process %d shared by %s and %s",provProc[i].pid,info->providerName,
+ provProc[i].firstProv->providerName));
+ if (provProc[i].firstProv) info->next=provProc[i].firstProv;
+ else info->next = NULL;
+ provProc[i].firstProv=info;
+ info->proc=provProc+i;
+ if (info->unload<provProc[i].unload) provProc[i].unload=info->unload;
+ if (semReleaseUnDo(sfcbSem,PROV_GUARD(provProc[i].id))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error releasing semaphore for %d, reason: %s\n",
+ provProc[i].id, strerror(errno));
+ _SFCB_ABORT();
+ }
+ _SFCB_RETURN(provProc[i].pid);
+ }
+ if (semReleaseUnDo(sfcbSem,PROV_GUARD(provProc[i].id))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error releasing semaphore for %d, reason: %s\n",
+ provProc[i].id, strerror(errno));
+ _SFCB_ABORT();
+ }
}
}
}
@@ -761,13 +834,41 @@
providerProcess=1;
info->proc=*proc;
info->pid=currentProc;
-
- semSetValue(sfcbSem,PROV_GUARD((*proc)->id),0);
- semSetValue(sfcbSem,PROV_INUSE((*proc)->id),0);
- semSetValue(sfcbSem,PROV_ALIVE((*proc)->id),0);
- semReleaseUnDo(sfcbSem,PROV_ALIVE((*proc)->id));
- semReleaseUnDo(sfcbSem,PROV_INUSE((*proc)->id));
- semRelease(sfcbSem,PROV_GUARD((*proc)->id));
+
+ /* The guard semaphore may never increase beyond 1, unless it is relased more often than
+ acquired. Therefore it is cleaner to acquire it than to set it to 0 unconditionally,
+ which can lead to a race, but we will also check the value to be sure.
+ */
+ if (semAcquireUnDo(sfcbSem,PROV_GUARD((*proc)->id))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error acquiring semaphore for %d, reason: %s\n",
+ (*proc)->id, strerror(errno));
+ _SFCB_ABORT();
+ }
+ if (semGetValue(sfcbSem,PROV_GUARD((*proc)->id))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error, guard semaphore for %d is not zero\n",
+ (*proc)->id);
+ _SFCB_ABORT();
+ }
+ if (semSetValue(sfcbSem,PROV_INUSE((*proc)->id),0)) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error resetting inuse semaphore for %d, reason: %s\n",
+ (*proc)->id, strerror(errno));
+ _SFCB_ABORT();
+ }
+ if (semSetValue(sfcbSem,PROV_ALIVE((*proc)->id),0)) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error resetting alive semaphore for %d, step 1, reason: %s\n",
+ (*proc)->id, strerror(errno));
+ _SFCB_ABORT();
+ }
+ if (semReleaseUnDo(sfcbSem,PROV_ALIVE((*proc)->id))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error resetting alive semaphore for %d, step 2, reason: %s\n",
+ (*proc)->id, strerror(errno));
+ _SFCB_ABORT();
+ }
+ if (semReleaseUnDo(sfcbSem,PROV_GUARD((*proc)->id))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error releasing semaphore for %d, reason: %s\n",
+ (*proc)->id, strerror(errno));
+ _SFCB_ABORT();
+ }
processProviderInvocationRequests(info->providerName);
_SFCB_RETURN(0);
@@ -797,17 +898,21 @@
if (info->pid ) {
proc=info->proc;
- semAcquire(sfcbSem,PROV_GUARD(proc->id));
- if ((val=semGetValue(sfcbSem,PROV_ALIVE(proc->id)))) {
- semRelease(sfcbSem,PROV_INUSE(proc->id));
- semRelease(sfcbSem,PROV_GUARD(proc->id));
- _SFCB_TRACE(1, ("--- Provider %s still loaded",info->providerName));
- _SFCB_RETURN(CMPI_RC_OK)
+ if (semAcquireUnDo(sfcbSem,PROV_GUARD(proc->id))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error acquiring semaphore for %d, reason: %s\n",
+ proc->id, strerror(errno));
+ _SFCB_ABORT();
+ }
+ if ((val=semGetValue(sfcbSem,PROV_ALIVE(proc->id))) > 0) {
+ if (semReleaseUnDo(sfcbSem,PROV_GUARD(proc->id))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error releasing semaphore for %d, reason: %s\n",
+ proc->id, strerror(errno));
+ _SFCB_ABORT();
+ }
+ _SFCB_TRACE(1, ("--- Provider %s still loaded",info->providerName));
+ _SFCB_RETURN(CMPI_RC_OK);
}
- semRelease(sfcbSem,PROV_GUARD(proc->id));
- _SFCB_TRACE(1, ("--- Provider has been unloaded prevously, will reload"));
-
info->pid=0;
for (pInfo=proc->firstProv; pInfo; pInfo=pInfo->next) {
pInfo->pid=0;
@@ -815,6 +920,14 @@
proc->firstProv=NULL;
proc->pid=0;
proc->group=NULL;
+
+ if (semReleaseUnDo(sfcbSem,PROV_GUARD(proc->id))) {
+ mlogf(M_ERROR,M_SHOW,"-#- Fatal error releasing semaphore for %d, reason: %s\n",
+ proc->id, strerror(errno));
+ _SFCB_ABORT();
+ }
+ _SFCB_TRACE(1, ("--- Provider has been unloaded prevously, will reload"));
+
}
_SFCB_TRACE(1, ("--- Forking provider for %s", info->providerName));
@@ -2252,8 +2365,9 @@
_SFCB_TRACE(1, ("--- Back from provider rc: %d", rci.rc));
if (rci.rc==CMPI_RC_OK) {
- resp = (BinResponseHdr *) calloc(1,sizeof(BinResponseHdr));
- resp->rc=1;
+ increaseInUseSem(info->provIds.procId);
+ resp = (BinResponseHdr *) calloc(1,sizeof(BinResponseHdr));
+ resp->rc=1;
}
}
@@ -2319,8 +2433,9 @@
}
TIMING_STOP(hdr,info)
if (rci.rc==CMPI_RC_OK) {
- resp->rc=1;
- _SFCB_RETURN(resp);
+ decreaseInUseSem(info->provIds.procId);
+ resp->rc=1;
+ _SFCB_RETURN(resp);
}
if (resp) free(resp);
@@ -2366,8 +2481,8 @@
}
if (rci.rc==CMPI_RC_OK) {
- resp = (BinResponseHdr *) calloc(1,sizeof(BinResponseHdr));
- resp->rc=1;
+ resp = (BinResponseHdr *) calloc(1,sizeof(BinResponseHdr));
+ resp->rc=1;
}
if (rci.rc!=CMPI_RC_OK) {
resp = errorResp(&rci);
Index: ChangeLog
===================================================================
RCS file: /cvsroot/sblim/sfcb/ChangeLog,v
retrieving revision 1.730
retrieving revision 1.731
diff -u -d -r1.730 -r1.731
--- ChangeLog 6 Mar 2012 17:33:55 -0000 1.730
+++ ChangeLog 7 Mar 2012 00:52:36 -0000 1.731
@@ -1,6 +1,10 @@
2012-03-06 Chris Buccella <buc...@li...>
* providerDrv.c, providerMgr.c:
+ [ 3497950 ] SFCB Semaphore Handling Improvements
+ (patch by Viktor Mihajlovski)
+
+ * providerDrv.c, providerMgr.c:
[ 3497765 ] Better Error Handling in sfcb Provider Manager and Driver
(patch by Viktor Mihajlovski)
|
