Re: [HACKERS] Question on win32 semaphore simulation

While we have installed a Win32-specific semaphore implementation for
CVS HEAD, what do we want do apply for the back branches, 8.0.X and
8.1.X. Is this the patch that should be applied?

---------------------------------------------------------------------------

Qingqing Zhou wrote:
>
> "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;
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: In versions below 8.0, the planner will ignore your desire to
> choose an index scan if your joining column's datatypes do not
> match
>

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +