Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitea8e1bb

Browse files
committed
Improve error detection capability in proclists.
Previously, although the initial state of a proclist_node is expectedto be next == prev == 0, proclist_delete_offset would reset nodes tonext == prev == INVALID_PGPROCNO when removing them from a list.This is the same state that a node in a singleton list has, so thatit's impossible to distinguish not-in-a-list from in-a-list. Changeproclist_delete_offset to reset removed nodes to next == prev == 0,making it possible to distinguish those cases, and then add Assertsto the list add and delete functions that the supplied node isn'tor is in a list at entry. Also tighten assertions about the nodebeing in the particular list (not some other one) where it is possibleto check that in O(1) time.In ConditionVariablePrepareToSleep, since we don't expect the process'scvWaitLink to already be in a list, remove the more-or-less-uselessproclist_contains check; we'd rather have proclist_push_tail's newassertion fire if that happens.Improve various comments related to proclists, too.Patch by me, reviewed by Thomas Munro. This isn't back-patchable, sincethere could theoretically be inlined copies of proclist_delete_offset inthird-party modules. But it's only improving debuggability anyway.Discussion:https://postgr.es/m/CAEepm=0NWKehYw7NDoUSf8juuKOPRnCyY3vuaSvhrEWsOTAa3w@mail.gmail.com
1 parenteeb3c2d commitea8e1bb

File tree

3 files changed

+43
-27
lines changed

3 files changed

+43
-27
lines changed

‎src/backend/storage/lmgr/condition_variable.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,7 @@ ConditionVariablePrepareToSleep(ConditionVariable *cv)
8686

8787
/* Add myself to the wait queue. */
8888
SpinLockAcquire(&cv->mutex);
89-
if (!proclist_contains(&cv->wakeup,pgprocno,cvWaitLink))
90-
proclist_push_tail(&cv->wakeup,pgprocno,cvWaitLink);
89+
proclist_push_tail(&cv->wakeup,pgprocno,cvWaitLink);
9190
SpinLockRelease(&cv->mutex);
9291
}
9392

‎src/include/storage/proclist.h

Lines changed: 33 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ proclist_is_empty(proclist_head *list)
4242

4343
/*
4444
* Get a pointer to a proclist_node inside a given PGPROC, given a procno and
45-
*anoffset.
45+
*the proclist_node field'soffset within struct PGPROC.
4646
*/
4747
staticinlineproclist_node*
4848
proclist_node_get(intprocno,size_tnode_offset)
@@ -53,13 +53,15 @@ proclist_node_get(int procno, size_t node_offset)
5353
}
5454

5555
/*
56-
* Insert anode at the beginning of a list.
56+
* Insert aprocess at the beginning of a list.
5757
*/
5858
staticinlinevoid
5959
proclist_push_head_offset(proclist_head*list,intprocno,size_tnode_offset)
6060
{
6161
proclist_node*node=proclist_node_get(procno,node_offset);
6262

63+
Assert(node->next==0&&node->prev==0);
64+
6365
if (list->head==INVALID_PGPROCNO)
6466
{
6567
Assert(list->tail==INVALID_PGPROCNO);
@@ -79,13 +81,15 @@ proclist_push_head_offset(proclist_head *list, int procno, size_t node_offset)
7981
}
8082

8183
/*
82-
* Insert anode at the end of a list.
84+
* Insert aprocess at the end of a list.
8385
*/
8486
staticinlinevoid
8587
proclist_push_tail_offset(proclist_head*list,intprocno,size_tnode_offset)
8688
{
8789
proclist_node*node=proclist_node_get(procno,node_offset);
8890

91+
Assert(node->next==0&&node->prev==0);
92+
8993
if (list->tail==INVALID_PGPROCNO)
9094
{
9195
Assert(list->head==INVALID_PGPROCNO);
@@ -105,58 +109,65 @@ proclist_push_tail_offset(proclist_head *list, int procno, size_t node_offset)
105109
}
106110

107111
/*
108-
* Delete anode. The nodemust be in the list.
112+
* Delete aprocess from a list --- itmust be in the list!
109113
*/
110114
staticinlinevoid
111115
proclist_delete_offset(proclist_head*list,intprocno,size_tnode_offset)
112116
{
113117
proclist_node*node=proclist_node_get(procno,node_offset);
114118

119+
Assert(node->next!=0||node->prev!=0);
120+
115121
if (node->prev==INVALID_PGPROCNO)
122+
{
123+
Assert(list->head==procno);
116124
list->head=node->next;
125+
}
117126
else
118127
proclist_node_get(node->prev,node_offset)->next=node->next;
119128

120129
if (node->next==INVALID_PGPROCNO)
130+
{
131+
Assert(list->tail==procno);
121132
list->tail=node->prev;
133+
}
122134
else
123135
proclist_node_get(node->next,node_offset)->prev=node->prev;
124136

125-
node->next=node->prev=INVALID_PGPROCNO;
137+
node->next=node->prev=0;
126138
}
127139

128140
/*
129-
* Check if anode is currently in a list. It must be known that the node is
130-
* not in any _other_ proclist that uses the same proclist_node, so that the
131-
* only possibilities are that it is in this list or none.
141+
* Check if aprocess is currently in a list. It must be known that the
142+
*process isnot in any _other_ proclist that uses the same proclist_node,
143+
*so that theonly possibilities are that it is in this list or none.
132144
*/
133145
staticinlinebool
134146
proclist_contains_offset(proclist_head*list,intprocno,
135147
size_tnode_offset)
136148
{
137149
proclist_node*node=proclist_node_get(procno,node_offset);
138150

139-
/*
140-
* If this is not a member of a proclist, then the next and prev pointers
141-
* should be 0. Circular lists are not allowed so this condition is not
142-
* confusable with a real pgprocno 0.
143-
*/
151+
/* If it's not in any list, it's definitely not in this one. */
144152
if (node->prev==0&&node->next==0)
145153
return false;
146154

147-
/* If there is a previous node, then this node must be in the list. */
148-
if (node->prev!=INVALID_PGPROCNO)
149-
return true;
150-
151155
/*
152-
* There is no previous node, so the only way this node can be in the list
153-
* is if it's the head node.
156+
* It must, in fact, be in this list. Ideally, in assert-enabled builds,
157+
* we'd verify that. But since this function is typically used while
158+
* holding a spinlock, crawling the whole list is unacceptable. However,
159+
* we can verify matters in O(1) time when the node is a list head or
160+
* tail, and that seems worth doing, since in practice that should often
161+
* be enough to catch mistakes.
154162
*/
155-
returnlist->head==procno;
163+
Assert(node->prev!=INVALID_PGPROCNO||list->head==procno);
164+
Assert(node->next!=INVALID_PGPROCNO||list->tail==procno);
165+
166+
return true;
156167
}
157168

158169
/*
159-
* Remove and return the firstnode from a list (there must be one).
170+
* Remove and return the firstprocess from a list (there must be one).
160171
*/
161172
staticinlinePGPROC*
162173
proclist_pop_head_node_offset(proclist_head*list,size_tnode_offset)
@@ -205,4 +216,4 @@ proclist_pop_head_node_offset(proclist_head *list, size_t node_offset)
205216
proclist_node_get((iter).cur,\
206217
offsetof(PGPROC, link_member))->next)
207218

208-
#endif
219+
#endif/* PROCLIST_H */

‎src/include/storage/proclist_types.h

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,12 @@
1616
#definePROCLIST_TYPES_H
1717

1818
/*
19-
* A node in a list of processes.
19+
* A node in a doubly-linked list of processes. The link fields contain
20+
* the 0-based PGPROC indexes of the next and previous process, or
21+
* INVALID_PGPROCNO in the next-link of the last node and the prev-link
22+
* of the first node. A node that is currently not in any list
23+
* should have next == prev == 0; this is not a possible state for a node
24+
* that is in a list, because we disallow circularity.
2025
*/
2126
typedefstructproclist_node
2227
{
@@ -25,7 +30,8 @@ typedef struct proclist_node
2530
}proclist_node;
2631

2732
/*
28-
* Head of a doubly-linked list of PGPROCs, identified by pgprocno.
33+
* Header of a doubly-linked list of PGPROCs, identified by pgprocno.
34+
* An empty list is represented by head == tail == INVALID_PGPROCNO.
2935
*/
3036
typedefstructproclist_head
3137
{
@@ -42,4 +48,4 @@ typedef struct proclist_mutable_iter
4248
intnext;/* pgprocno of the next PGPROC */
4349
}proclist_mutable_iter;
4450

45-
#endif
51+
#endif/* PROCLIST_TYPES_H */

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp