Menu
▾
▴
[Sblim-commit] Git: SFCB - Small Footprint CIM Broker branch, master, updated. SFCB_1_4_0-191-g0d24425
|
From: Chris B. <buc...@us...> - 2012-03-06 22:52:39
|
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "SFCB - Small Footprint CIM Broker".
The branch, master has been updated
via 0d2442510d80a84be391c1ccdfccd540c3296c13 (commit)
from c7aea3d3eeda8b9d80fea5f14b0e00e18fdd73db (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 0d2442510d80a84be391c1ccdfccd540c3296c13
Author: buccella <buc...@li...>
Date: Tue Mar 6 17:53:13 2012 -0500
[ 3497950 ] SFCB Semaphore Handling Improvements
-----------------------------------------------------------------------
Summary of changes:
diff --git a/ChangeLog b/ChangeLog
index afdb7c0..63fa2d8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -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)
diff --git a/NEWS b/NEWS
index 26ff00c..f394344 100644
--- a/NEWS
+++ b/NEWS
@@ -107,6 +107,7 @@ Bugs Fixed:
- 3496061 IndicationSubscription May Be Undeletable
- 3496383 Faster Return from CBDeliverIndication
- 3497765 Better Error Handling in sfcb Provider Manager and Driver
+- 3497950 SFCB Semaphore Handling Improvements
Changes in 1.3.13
=================
diff --git a/providerDrv.c b/providerDrv.c
index d0b202b..f7f1e2e 100644
--- a/providerDrv.c
+++ b/providerDrv.c
@@ -161,6 +161,9 @@ static int idleThreadStartHandled = 0;
ProviderInfo *activProvs = NULL;
+static void increaseInUseSem(int id);
+static void decreaseInUseSem(int id);
+
unsigned long provSampleInterval = 10;
unsigned long provTimeoutInterval = 25;
unsigned provAutoGroup = 0;
@@ -372,6 +375,52 @@ stopNextProc()
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
@@ -565,8 +614,12 @@ providerIdleThread()
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) {
/* providerTimeoutInterval reached? */
if ((now - proc->lastActivity) > provTimeoutInterval) {
ctx = native_new_CMPIContext(MEM_TRACKED, NULL);
@@ -654,7 +707,11 @@ providerIdleThread()
}
}
}
- 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();
+ }
}
}
}
@@ -824,24 +881,35 @@ getProcess(ProviderInfo * info, ProviderProcess ** proc)
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();
+ }
}
}
}
@@ -902,12 +970,40 @@ getProcess(ProviderInfo * info, ProviderProcess ** proc)
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);
@@ -939,17 +1035,20 @@ forkProvider(ProviderInfo * info, char **msg)
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) {
@@ -958,6 +1057,13 @@ forkProvider(ProviderInfo * info, char **msg)
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));
@@ -2635,6 +2741,7 @@ activateFilter(BinRequestHdr * hdr, ProviderInfo * info, int requestor)
_SFCB_TRACE(1, ("--- Back from provider rc: %d", rci.rc));
if (rci.rc == CMPI_RC_OK) {
+ increaseInUseSem(info->provIds.procId);
resp = (BinResponseHdr *) calloc(1, sizeof(BinResponseHdr));
resp->rc = 1;
}
@@ -2716,7 +2823,8 @@ deactivateFilter(BinRequestHdr * hdr, ProviderInfo * info, int requestor)
"", path, 1);
}
TIMING_STOP(hdr, info)
- if (rci.rc == CMPI_RC_OK) {
+ if (rci.rc == CMPI_RC_OK) {
+ decreaseInUseSem(info->provIds.procId);
resp->rc = 1;
_SFCB_RETURN(resp);
}
diff --git a/providerMgr.c b/providerMgr.c
index ff29cc0..a075af2 100644
--- a/providerMgr.c
+++ b/providerMgr.c
@@ -1052,15 +1052,27 @@ closeProviderContext(BinRequestContext * ctx)
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);
@@ -1075,15 +1087,30 @@ setInuseSem(void *id)
_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();
}
hooks/post-receive
--
SFCB - Small Footprint CIM Broker
|
