Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Shruthi Gowda <gowdashru@gmail.com>
From: Shruthi Gowda <gowdashru@gmail.com>
To: Nishant Sharma <nishant.sharma@enterprisedb.com>
Cc: Mahendra Singh Thalor <mahi6run@gmail.com>,
Fujii Masao <masao.fujii@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>,
PostgreSQL Development <pgsql-hackers@postgresql.org>
Date: 2026-04-09T10:44:13Z
Lists: pgsql-hackers
Attachments
- v4-0001-Add-missing-connection-validation-in-ECPG.patch (application/octet-stream) patch v4-0001
- v1_test-0001-Tests-for-NULL-connection-validation.patch (application/octet-stream) patch v1-0001
On Tue, Mar 24, 2026 at 11:29 AM Nishant Sharma < nishant.sharma@enterprisedb.com> wrote: > Here are some review comments on v3 patch:- > > 1. > > Change in descriptor.c file - In my opinion, we can use `if(conn)` > with ecpg_raise, like other occurrence of ecpg_get_connection() call check > in this file, and not using ecpg_init(). Three reasons: a) Consistency in > checking conn after ecpg_get_connection() call in this file with if check. > b) We don't need to remove 'ecpg_init_sqlca(sqlca);' line due to call to > ecpg_init(). c) #2 comment below. > 2. > > If you agree with #1, then I see many other reasons for which > ECPGget_desc() returns and we can avoid ecpg_get_connection() call at top > of that function for those reasons and keep the check at the required > location only instead of moving at top of the function. > 3. > > I see there is one more location of ecpg_get_connection() call where > there is no check of NULL conn. In function ecpg_freeStmtCacheEntry() of > file prepare.c? I understand it's not required for a call in > ecpg_auto_prepare(), as the caller already validated that connection > string. But I think, conn in ecpg_freeStmtCacheEntry() is different from > the one that was validated. > 4. > > +1 to Mahindra, new test cases specific to the crash required for this > change? > > > > Regards, > Nishant Sharma, > EDB, Pune. > https://www.enterprisedb.com/ > Thanks, Nishant, for the review. I agree with points 1 and 2 and have revised the patch accordingly. Regarding point 3, you are correct; the conn in ecpg_freeStmtCacheEntry() differs from the one validated in the caller. I have now added the necessary validation in the latest version. I have also included a test case patch covering all execution paths except for the ecpg_freeStmtCacheEntry() flow. I was unable to trigger that specific flow, and it appears none of the existing test cases cover that line either. Thanks & Regards, Shruthi K C EnterpriseDB: http://www.enterprisedb.com
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Add missing connection validation in ECPG
- f0e3f9732d41 14.23 landed
- 6916f4410051 15.18 landed
- 7e4c871f4181 16.14 landed
- 5d67549d9412 17.10 landed
- e2688ea5e411 18.4 landed
- c34a280c85b3 19 (unreleased) landed