Re: [HACKERS] Question on win32 semaphore simulation - Mailing list pgsql-patches

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@cs.toronto.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. +