Re: Question on win32 semaphore simulation - Mailing list pgsql-hackers

"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;