Re: [devel] Re: I: Sisyphus-20050929 unmets: +2 (93/40)

[Vladimir Lettiev <crux@gorodmasterov.com>]

[-- Attachment #1: Type: text/plain, Size: 1547 bytes --]

Dmitry V. Levin пишет:
> On Sun, Oct 02, 2005 at 02:19:07PM +0400, Vladimir Lettiev wrote:
>>Я сделал патч для IPC::ShareLite.
>>Как мне показалось проблема в том, что данный модуль сначала делает 
>>shmdt() и лишь после пытается метить сегмент флагом IPC_RMID. Попробовал 
>>поменять порядок и вроде как заработало.
>>Если кто-то может его прокомментировать, welcome.

Как оказалось проблема была в том, что происходила попытка подключения к 
сегменту, который был удалён.

> Патч без контекста не очевиден, но то, что вы предлагаете сделать, логично
> и, насколько я понимаю, должно работать на всех ядрах.

Сложность в том, что IPC::ShareLite позволяет создавать сегмент, который 
может как удаляться, после отработки программы, так и нет (зачем это 
надо, не знаю). Видимо придётся фиксить также все программы, которые 
захотят использовать последнюю возможность этого модуля.

Патч я переделал. Теперь модуль проходит как собственные тесты, так и 
тесты других приложений, которые его используют (например, 
perl-Cache-Cache). Возможно ошибки всё же всплывут, но по крайне мере 
удалось исключить зависание, за счёт использования вместо semop() вызова 
semtimedop(), который позволяет задать время таймаута.

Смысл патча^Wхака в том, чтобы не детачится от сегмента, если число 
подключений меньше двух. И проверять перед унижтожением сегмента, что он 
уже не был уничтожен раньше, чтобы не нервировать shmat().

p.s. Исправленый пакет с этим патчем я уже залил в incoming.

-- 
С уважением, Владимир Леттиев aka crux <crux@gorodmasterov.com>

[-- Attachment #2: perl-IPC-ShareLite-fixOwl.patch --]
[-- Type: text/x-patch, Size: 2304 bytes --]

--- sharelite.c~~	2000-04-14 14:24:00 +0400
+++ sharelite.c	2005-10-03 22:35:59 +0400
@@ -22,12 +22,12 @@
 
 /* --- DEFINE MACROS FOR SEMAPHORE OPERATIONS --- */
 
-#define GET_EX_LOCK(A)    semop((A), &ex_lock[0],    3)
-#define GET_EX_LOCK_NB(A) semop((A), &ex_lock_nb[0], 3)
-#define RM_EX_LOCK(A)     semop((A), &ex_unlock[0],  1)
-#define GET_SH_LOCK(A)    semop((A), &sh_lock[0],    2)
-#define GET_SH_LOCK_NB(A) semop((A), &sh_lock_nb[0], 2)
-#define RM_SH_LOCK(A)     semop((A), &sh_unlock[0],  1) 
+#define GET_EX_LOCK(A)    semtimedop((A), &ex_lock[0],    3, &timeout[0])
+#define GET_EX_LOCK_NB(A) semtimedop((A), &ex_lock_nb[0], 3, &timeout[0])
+#define RM_EX_LOCK(A)     semtimedop((A), &ex_unlock[0],  1, &timeout[0])
+#define GET_SH_LOCK(A)    semtimedop((A), &sh_lock[0],    2, &timeout[0])
+#define GET_SH_LOCK_NB(A) semtimedop((A), &sh_lock_nb[0], 2, &timeout[0])
+#define RM_SH_LOCK(A)     semtimedop((A), &sh_unlock[0],  1, &timeout[0])
 
 /* --- DEFINE STRUCTURES FOR MANIPULATING SEMAPHORES --- */
 
@@ -61,6 +61,10 @@
   { 1, -1, (SEM_UNDO | IPC_NOWAIT) } /* remove shared read lock */
 };                                 
 
+static struct timespec timeout[1] = {
+  { 3, 0 } /* 3 sec timeout */
+};
+
 /* USER INITIATED LOCK */
 
 /* returns 0  on success -- requested operation performed   *
@@ -192,10 +196,14 @@
 
 int _detach_segments(Node *node) {
   Node *next_node;
+  struct shmid_ds shmarg;
 
   while(node != NULL) {
     next_node  = node->next;
-    if (shmdt((char *) node->shmaddr) < 0) return -1;
+    if (shmctl(node->shmid,IPC_STAT, &shmarg) < 0) return -1;
+    if (shmarg.shm_nattch > 1) {
+      if (shmdt((char *) node->shmaddr) < 0) return -1;
+    }
     free(node);
     node = next_node;
   }
@@ -205,12 +213,14 @@
 int _remove_segments(int shmid) {
   int    next_shmid;
   Header *shmaddr;
+  struct shmid_ds shmarg;
 
   while(shmid >= 0) {
+    if (shmctl(shmid, IPC_STAT, &shmarg) < 0 && errno == EIDRM) return 0;
     if ((shmaddr = (Header *) shmat(shmid, (char *) 0, 0)) == (Header *) -1) return -1;
     next_shmid = shmaddr->next_shmid;
-    if (shmdt((char *) shmaddr) < 0) return -1;   
     if (shmctl(shmid, IPC_RMID, (struct shmid_ds *) 0) < 0) return -1;
+    if (shmdt((char *) shmaddr) < 0) return -1;   
     shmid = next_shmid;
   }