Re: Question on win32 semaphore simulation

"Qingqing Zhou" <zhouqq(at)cs(dot)toronto(dot)edu> wrote
>
> So we might want to fix current win32/sema.c for two problems:
> (1) semctl(SETVAL, val=0) - there is no other "val" than zero is used;
> (2) concurrent access to sem_counts[];
>

Attached is a patch for the above proposed change -- but still, I hope we
don't need semctl(SETVAL) at all.

Regards,
Qingqing

---------

Index: sema.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/port/win32/sema.c,v
retrieving revision 1.13
diff -c -r1.13 sema.c
*** sema.c 9 Apr 2006 19:21:34 -0000 1.13
--- sema.c 19 Apr 2006 03:56:55 -0000
***************
*** 42,48 ****
/* Fix the count of all sem of the pool to semun.array */
if (flag == SETALL)
{
! int i;
struct sembuf sops;

sops.sem_flg = IPC_NOWAIT;
--- 42,49 ----
/* Fix the count of all sem of the pool to semun.array */
if (flag == SETALL)
{
! int i;
! int errStatus;
struct sembuf sops;

sops.sem_flg = IPC_NOWAIT;
***************
*** 60,66 ****
sops.sem_num = i;

/* Quickly lock/unlock the semaphore (if we can) */
! if (semop(semId, &sops, 1) < 0)
return -1;
}
return 1;
--- 61,72 ----
sops.sem_num = i;

/* Quickly lock/unlock the semaphore (if we can) */
! do
! {
! errStatus = semop(semId, &sops, 1);
! } while (errStatus < 0 && errno == EINTR);
!
! if (errStatus < 0)
return -1;
}
return 1;
***************
*** 72,88 ****
if (semun.val != sem_counts[semNum])
{
struct sembuf sops;

sops.sem_flg = IPC_NOWAIT;
sops.sem_num = semNum;
!
! if (semun.val < sem_counts[semNum])
! sops.sem_op = -1;
! else
! sops.sem_op = 1;

/* Quickly lock/unlock the semaphore (if we can) */
! if (semop(semId, &sops, 1) < 0)
return -1;
}

--- 78,96 ----
if (semun.val != sem_counts[semNum])
{
struct sembuf sops;
+ int errStatus;

sops.sem_flg = IPC_NOWAIT;
sops.sem_num = semNum;
! sops.sem_op = semun.val - sem_counts[semNum];

/* Quickly lock/unlock the semaphore (if we can) */
! do
! {
! errStatus = semop(semId, &sops, 1);
! } while (errStatus < 0 && errno == EINTR);
!
! if (errStatus < 0)
return -1;
}

***************
*** 226,269 ****

cur_handle = sem_handles[sops[0].sem_num];

! if (sops[0].sem_op == -1)
{
DWORD ret;
HANDLE wh[2];

wh[0] = cur_handle;
wh[1] = pgwin32_signal_event;

! ret = WaitForMultipleObjectsEx(2, wh, FALSE, (sops[0].sem_flg &
IPC_NOWAIT) ? 0 : INFINITE, TRUE);
!
! if (ret == WAIT_OBJECT_0)
{
! /* We got it! */
! sem_counts[sops[0].sem_num]--;
! return 0;
}
! else if (ret == WAIT_OBJECT_0 + 1 || ret == WAIT_IO_COMPLETION)
! {
! /* Signal event is set - we have a signal to deliver */
! pgwin32_dispatch_queued_signals();
! errno = EINTR;
! }
! else if (ret == WAIT_TIMEOUT)
! /* Couldn't get it */
! errno = EAGAIN;
! else
! errno = EIDRM;
}
! else if (sops[0].sem_op > 0)
{
/* Don't want the lock anymore */
! sem_counts[sops[0].sem_num]++;
ReleaseSemaphore(cur_handle, sops[0].sem_op, NULL);
return 0;
}
- else
- /* Not supported */
- errno = ERANGE;

/* If we get down here, then something is wrong */
return -1;
--- 234,295 ----

cur_handle = sem_handles[sops[0].sem_num];

! if (sops[0].sem_op < 0)
{
DWORD ret;
HANDLE wh[2];
+ int i;

wh[0] = cur_handle;
wh[1] = pgwin32_signal_event;

! /*
! * Try to consume the specified sem count. If we can't, we just
! * quit the operation silently because it is possible there is
! * another process just did some semop(-k) during our loop.
! */
! errno = 0;
! for (i = 0; i < -(sops[0].sem_op); i++)
{
! ret = WaitForMultipleObjectsEx(2, wh, FALSE,
! (sops[0].sem_flg & IPC_NOWAIT) ? 0 : INFINITE, TRUE);
!
! if (ret == WAIT_OBJECT_0)
! {
! /* We got it! */
! InterlockedDecrement((volatile long *)(sem_counts + sops[0].sem_num));
! }
! else if (ret == WAIT_OBJECT_0 + 1 || ret == WAIT_IO_COMPLETION)
! {
! /* Signal event is set - we have a signal to deliver */
! pgwin32_dispatch_queued_signals();
! errno = EINTR;
! }
! else if (ret == WAIT_TIMEOUT)
! /* Couldn't get it */
! break;
! else
! errno = EIDRM;
!
! /* return immediately on error */
! if (errno != 0)
! break;
}
!
! /* successfully done */
! if (errno == 0)
! return 0;
}
! else
{
+ Assert(sops[0].sem_op > 0);
+
/* Don't want the lock anymore */
! InterlockedExchangeAdd((volatile long *)
! (sem_counts + sops[0].sem_num), sops[0].sem_op);
ReleaseSemaphore(cur_handle, sops[0].sem_op, NULL);
return 0;
}

/* If we get down here, then something is wrong */
return -1;