<div dir="ltr"><div dir="ltr">Op do 4 apr 2024 om 03:22 schreef Tatsuo Ishii <<a href="mailto:ishii@sraoss.co.jp">ishii@sraoss.co.jp</a>>:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">> I dove into the code, and I think I've found the cause of the error. Just<br>
> prior to crashing, it reports "find_primary_node:<br>
> make_persistent_db_connection_noerror failed on node 0". This must come<br>
> from pgpool_main.c:2782. This means that slots[0] is NULL. Then, at<br>
> pgpool_main.c:2791 it enters verify_backend_node_status with this slots<br>
> array. At lines 2569-2579 it loops over these slots,<br>
> calling get_server_version for every slot, including slots[0], which is<br>
> NULL. This crashes when get_server_version calls get_query_result, which<br>
> tries to dereference slots[0]->con. At pgpool_main.c:2456 there is an<br>
> explicit check for NULL, this is missing in the other for loop, but it is<br>
> also missing at line 2609.<br>
<br>
But there's a check at line 2604 of pgpool_main.c:<br>
<br>
if (pool_node_status[j] == POOL_NODE_STATUS_STANDBY)<br>
<br>
If pool_node_status[j] is POOL_NODE_STATUS_STANDBY, the target node<br>
(0) must be alive in the past. I suspect node 0 goes down after the<br>
pool_node_status[j] was updated. I should have checked slots<br>
availability before calling get_query_result at 2609.<br></blockquote><div><br></div><div>I wasn't sure about line 2609, but adding a check does make sense. The loop at lines 2569-2579 definitely is broken. This also is where the segfault happens at this moment. I've attached a patch (against 4.5.1) that should address this issue.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
As of crash in health_check.c, I think I have found the cause.  The<br>
connection info is cached in HealthCheckMemoryContext, which is<br>
pointed to by "slot" (a static variable). When an error occurred,<br>
ereport(ERROR) jumps to line 159. Then the code proceeds to the for<br>
loop starting at line 171. At line 174<br>
MemoryContextResetAndDeleteChildren(HealthCheckMemoryContext) is<br>
called and the connection info is discarded [1]. Problem is, the value<br>
of "slot" remains, which means that slot points to freed memory. We<br>
should have cleared slot there.<br>
<br>
Same issue is found in pool_worker_child.c.<br>
<br>
Attached is the patch for the above.<br></blockquote><div><br></div><div>Great. I've added the patch to our build, including the attached patch and I'm rerunning the tests. I did have to alter the patch for pool_worker_child.c a bit to make it apply on 4.5.1. </div><div> </div><div>Best regards,</div><div>Emond</div></div></div>