Thread
Commits
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
-
[BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Shruthi Gowda <gowdashru@gmail.com> — 2025-12-08T15:38:56Z
Hi, The ECPG application crashes with a segmentation fault when calling specific deallocation or prepared statement functions without an established database connection. This is caused by a missing NULL check on the connection handle before attempting to access it. The issue is reproducible on the MASTER branch and affects older versions up to v13. The issue can be reproduced by modifying existing test cases, such as src/interfaces/ecpg/test/sql/execute.pgc or src/interfaces/ecpg/test/sql/desc.pgc. The general method is to skip the EXEC SQL CONNECT statement (or intentionally force the connection to fail). To specifically isolate the crash in ECPGdeallocate_all() and prevent the program from hitting the ECPGprepared_statement() crash first, modify the desc.pgc test file by moving the EXEC SQL DEALLOCATE ALL statement to an earlier point *Seg fault in ECPGprepared_statement():* [434574]: ECPGdebug: set to 1 [434574]: ECPGconnect: opening database ecpg1_regression on <DEFAULT> port <DEFAULT> [434574]: ECPGconnect: connection to server on socket "/tmp/.s.PGSQL.5444" failed: FATAL: database "ecpg1_regression" does not exist [434574]: ecpg_finish: connection main closed [434574]: raising sqlcode -402 on line 24: could not connect to database "ecpg1_regression" on line 24 SQL error: could not connect to database "ecpg1_regression" on line 24 [434574]: raising sqlcode -220 on line 25: connection "NULL" does not exist on line 25 SQL error: connection "NULL" does not exist on line 25 [434574]: raising sqlcode -220 on line 26: connection "NULL" does not exist on line 26 SQL error: connection "NULL" does not exist on line 26 [434574]: raising sqlcode -220 on line 29: connection "NULL" does not exist on line 29 SQL error: connection "NULL" does not exist on line 29 [434574]: raising sqlcode -220 on line 32: connection "NULL" does not exist on line 32 SQL error: connection "NULL" does not exist on line 32 [434574]: raising sqlcode -220 on line 35: connection "NULL" does not exist on line 35 SQL error: connection "NULL" does not exist on line 35 Inserted 0 tuples via execute immediate [434574]: raising sqlcode -220 on line 40: connection "NULL" does not exist on line 40 SQL error: connection "NULL" does not exist on line 40 [434574]: raising sqlcode -220 on line 41: connection "NULL" does not exist on line 41 SQL error: connection "NULL" does not exist on line 41 Inserted 0 tuples via prepared execute [434574]: raising sqlcode -220 on line 45: connection "NULL" does not exist on line 45 SQL error: connection "NULL" does not exist on line 45 [434574]: raising sqlcode -220 on line 49: connection "NULL" does not exist on line 49 SQL error: connection "NULL" does not exist on line 49 Program received signal SIGSEGV, Segmentation fault. 0x0000fffff7f92654 in ecpg_find_prepared_statement (name=0xaaaaaaaa1aa8 "f", con=0x0, prev_=0x0) at prepare.c:277 277 for (this = con->prep_stmts, prev = NULL; (gdb) bt #0 0x0000fffff7f92654 in ecpg_find_prepared_statement (name=0xaaaaaaaa1aa8 "f", con=0x0, prev_=0x0) at prepare.c:277 #1 0x0000fffff7f92aa8 in ecpg_prepared (name=0xaaaaaaaa1aa8 "f", con=0x0) at prepare.c:393 #2 0x0000fffff7f92afc in ECPGprepared_statement (connection_name=0x0, name=0xaaaaaaaa1aa8 "f", lineno=53) at prepare.c:404 #3 0x0000aaaaaaaa0f74 in main () at /home/shrkc/work/postgres/inst/bin/execute.pgc:52 *Seg fault in ECPGdeallocate_all() :* Program received signal SIGSEGV, Segmentation fault. 0x0000fffff7f92a2c in ecpg_deallocate_all_conn (lineno=25, c=ECPG_COMPAT_PGSQL, con=0x0) at prepare.c:372 372 while (con->prep_stmts) (gdb) bt #0 0x0000fffff7f92a2c in ecpg_deallocate_all_conn (lineno=25, c=ECPG_COMPAT_PGSQL, con=0x0) at prepare.c:372 #1 0x0000fffff7f92a78 in ECPGdeallocate_all (lineno=25, compat=0, connection_name=0x0) at prepare.c:384 #2 0x0000aaaaaaaa0e60 in main () at /home/shrkc/work/postgres/inst/bin/desc.pgc:25 Please find the proposed patch attached below for your review. Thanks & Regards, Shruthi K C EnterpriseDB: http://www.enterprisedb.com -
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Tom Lane <tgl@sss.pgh.pa.us> — 2025-12-08T16:09:55Z
Shruthi Gowda <gowdashru@gmail.com> writes: > The ECPG application crashes with a segmentation fault when calling > specific deallocation or prepared statement functions without an > established database connection. This is caused by a missing NULL check on > the connection handle before attempting to access it. Hmm ... poking around, I see several other places that aren't checking the result of ecpg_get_connection. Shouldn't we tighten them all? regards, tom lane
-
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Shruthi Gowda <gowdashru@gmail.com> — 2026-01-07T18:00:27Z
On Mon, Dec 8, 2025 at 9:39 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Shruthi Gowda <gowdashru@gmail.com> writes: > > The ECPG application crashes with a segmentation fault when calling > > specific deallocation or prepared statement functions without an > > established database connection. This is caused by a missing NULL check > on > > the connection handle before attempting to access it. > > Hmm ... poking around, I see several other places that aren't checking > the result of ecpg_get_connection. Shouldn't we tighten them all? > > regards, tom lane > I agree. I’ve reviewed all occurrences of ecpg_get_connection() and noted that, in most instances, it is followed by ecpg_init(), which validates the connection and returns immediately if the connection is NULL. In a few cases, the caller had already validated the connection. However, I identified an additional case that lacked this check, so I have revised the patch to include the missing validation. Thanks & Regards, Shruthi K C EnterpriseDB: http://www.enterprisedb.com
-
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Fujii Masao <masao.fujii@gmail.com> — 2026-01-08T16:02:31Z
On Thu, Jan 8, 2026 at 3:00 AM Shruthi Gowda <gowdashru@gmail.com> wrote: > > > On Mon, Dec 8, 2025 at 9:39 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> >> Shruthi Gowda <gowdashru@gmail.com> writes: >> > The ECPG application crashes with a segmentation fault when calling >> > specific deallocation or prepared statement functions without an >> > established database connection. This is caused by a missing NULL check on >> > the connection handle before attempting to access it. >> >> Hmm ... poking around, I see several other places that aren't checking >> the result of ecpg_get_connection. Shouldn't we tighten them all? >> >> regards, tom lane > > > I agree. I’ve reviewed all occurrences of ecpg_get_connection() and noted that, in most instances, it is followed by ecpg_init(), which validates the connection and returns immediately if the connection is NULL. Why did you add this check instead of calling ecpg_init()? Wouldn't it be better and sufficient to use ecpg_init() to validate the connection? + con = ecpg_get_connection(connection_name); + if (!con) + { + ecpg_raise(lineno, ECPG_NO_CONN, ECPG_SQLSTATE_CONNECTION_DOES_NOT_EXIST, + connection_name ? connection_name : ecpg_gettext("NULL")); Regards, -- Fujii Masao -
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Shruthi Gowda <gowdashru@gmail.com> — 2026-01-19T12:08:29Z
On Thu, Jan 8, 2026 at 9:32 PM Fujii Masao <masao.fujii@gmail.com> wrote: > On Thu, Jan 8, 2026 at 3:00 AM Shruthi Gowda <gowdashru@gmail.com> wrote: > > > > > > On Mon, Dec 8, 2025 at 9:39 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > >> > >> Shruthi Gowda <gowdashru@gmail.com> writes: > >> > The ECPG application crashes with a segmentation fault when calling > >> > specific deallocation or prepared statement functions without an > >> > established database connection. This is caused by a missing NULL > check on > >> > the connection handle before attempting to access it. > >> > >> Hmm ... poking around, I see several other places that aren't checking > >> the result of ecpg_get_connection. Shouldn't we tighten them all? > >> > >> regards, tom lane > > > > > > I agree. I’ve reviewed all occurrences of ecpg_get_connection() and > noted that, in most instances, it is followed by ecpg_init(), which > validates the connection and returns immediately if the connection is NULL. > > Why did you add this check instead of calling ecpg_init()? > Wouldn't it be better and sufficient to use ecpg_init() to validate > the connection? > > + con = ecpg_get_connection(connection_name); > + if (!con) > + { > + ecpg_raise(lineno, ECPG_NO_CONN, ECPG_SQLSTATE_CONNECTION_DOES_NOT_EXIST, > + connection_name ? connection_name : ecpg_gettext("NULL")); > > Thanks for the feedback, Fujii. I agree—using ecpg_init() is a more consistent approach and aligns with how this is handled in other parts of the code. I have updated the patch to use ecpg_init() for validation. Please find the revised version attached. The patch works for MASTER and all the back branches. Thanks & Regards, Shruthi K C EnterpriseDB: http://www.enterprisedb.com -
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Mahendra Singh Thalor <mahi6run@gmail.com> — 2026-03-24T02:34:01Z
On Mon, 19 Jan 2026 at 17:38, Shruthi Gowda <gowdashru@gmail.com> wrote: > > > > On Thu, Jan 8, 2026 at 9:32 PM Fujii Masao <masao.fujii@gmail.com> wrote: >> >> On Thu, Jan 8, 2026 at 3:00 AM Shruthi Gowda <gowdashru@gmail.com> wrote: >> > >> > >> > On Mon, Dec 8, 2025 at 9:39 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> >> >> >> Shruthi Gowda <gowdashru@gmail.com> writes: >> >> > The ECPG application crashes with a segmentation fault when calling >> >> > specific deallocation or prepared statement functions without an >> >> > established database connection. This is caused by a missing NULL check on >> >> > the connection handle before attempting to access it. >> >> >> >> Hmm ... poking around, I see several other places that aren't checking >> >> the result of ecpg_get_connection. Shouldn't we tighten them all? >> >> >> >> regards, tom lane >> > >> > >> > I agree. I’ve reviewed all occurrences of ecpg_get_connection() and noted that, in most instances, it is followed by ecpg_init(), which validates the connection and returns immediately if the connection is NULL. >> >> Why did you add this check instead of calling ecpg_init()? >> Wouldn't it be better and sufficient to use ecpg_init() to validate >> the connection? >> >> + con = ecpg_get_connection(connection_name); >> + if (!con) >> + { >> + ecpg_raise(lineno, ECPG_NO_CONN, ECPG_SQLSTATE_CONNECTION_DOES_NOT_EXIST, >> + connection_name ? connection_name : ecpg_gettext("NULL")); >> > > Thanks for the feedback, Fujii. I agree—using ecpg_init() is a more consistent approach and aligns with how this is handled in other parts of the code. > I have updated the patch to use ecpg_init() for validation. Please find the revised version attached. > The patch works for MASTER and all the back branches. > > Thanks & Regards, > > Shruthi K C > > EnterpriseDB: http://www.enterprisedb.com Thanks Shruthi for the updated patch. Please add a crash test case in your patch. If possible, please add a test for connection=NULL for ECPGdeallocate_all, ECPGprepared_statement and ECPGget_desc. -- Thanks and Regards Mahendra Singh Thalor EnterpriseDB: http://www.enterprisedb.com -
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Nishant Sharma <nishant.sharma@enterprisedb.com> — 2026-03-24T05:59:31Z
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/
-
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Shruthi Gowda <gowdashru@gmail.com> — 2026-04-09T10:44:13Z
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
-
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Nishant Sharma <nishant.sharma@enterprisedb.com> — 2026-04-13T10:30:00Z
Thanks Shruthi, for the new patches! Here are some review comments:- 1. I think we should rename "test_null_connection.pgc" to "test6.pgc", and make other required changes appropriately. We can see existing test files uses test1.pgc, test2.pgc ..., test5.pgc as naming convention, and has test description inside the file. 2. No need to add test_null_connection.c as commit file in src/interfaces/ecpg/test/connect/ folder, we don't see such C src files for other existing test files like test1.c, test2.c ..., test5.c. It is part of src/interfaces/ecpg/test/connect/.gitignore. So, need to add test6.c and its executable test6 like others in src/interfaces/ecpg/test/connect/.gitignore 3. After doing #2 above make sure to create the test9.c C src file as src/interfaces/ecpg/test/expected/connect-test6.c, I can see other existing connect tests C src files in src/interfaces/ecpg/test/expected/ folder. 3. Not able to run "make -j 8 installcheck" on my setup, fails due to #3. 4. Extra space at the end (don't see that in other tested cases prints) : printf("Test 1: Try to get descriptor on a disconnected connection \n"); 5. char *query = "SELECT 1" -- unused in new test file added. 6. "if (stmt.connection == NULL)" --> "if (!stmt.connection)"? 7. Should we return -1 in else of new fix added in ecpg_freeStmtCacheEntry()? If 'con' is NULL, why clean up the cache? Is returning -1 more defensive than cleaning the cache? 8. Do we need to clean up "stmt.oldlocale = ecpg_strdup(...)" and undo setlocale() before returning from the newly added fix in ECPGget_desc()? Regards, Nishant Sharma, EDB, Pune. https://www.enterprisedb.com/ On Thu, Apr 9, 2026 at 4:14 PM Shruthi Gowda <gowdashru@gmail.com> wrote: > > > 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 > > -
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Nishant Sharma <nishant.sharma@enterprisedb.com> — 2026-04-13T10:32:24Z
Apologies, I top posted my previous review comments.
-
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Shruthi Gowda <gowdashru@gmail.com> — 2026-04-15T09:56:27Z
On Mon, Apr 13, 2026 at 4:00 PM Nishant Sharma < nishant.sharma@enterprisedb.com> wrote: > Thanks Shruthi, for the new patches! > > > Here are some review comments:- > 1. I think we should rename "test_null_connection.pgc" to "test6.pgc", and > make other required changes appropriately. We can see existing test files > uses test1.pgc, test2.pgc ..., test5.pgc as naming convention, and has test > description inside the file. > 2. No need to add test_null_connection.c as commit file in > src/interfaces/ecpg/test/connect/ folder, we don't see such C src files for > other existing test files like test1.c, test2.c ..., test5.c. It is part of > src/interfaces/ecpg/test/connect/.gitignore. So, need to add test6.c and > its executable test6 like others in > src/interfaces/ecpg/test/connect/.gitignore > 3. After doing #2 above make sure to create the test9.c C src file as > src/interfaces/ecpg/test/expected/connect-test6.c, I can see other existing > connect tests C src files in src/interfaces/ecpg/test/expected/ folder. > 3. Not able to run "make -j 8 installcheck" on my setup, fails due to #3. > 4. Extra space at the end (don't see that in other tested cases prints) : > printf("Test 1: Try to get descriptor on a disconnected connection \n"); > 5. char *query = "SELECT 1" -- unused in new test file added. > 6. "if (stmt.connection == NULL)" --> "if (!stmt.connection)"? > 7. Should we return -1 in else of new fix added in > ecpg_freeStmtCacheEntry()? If 'con' is NULL, why clean up the cache? Is > returning -1 more defensive than cleaning the cache? > 8. Do we need to clean up "stmt.oldlocale = ecpg_strdup(...)" and undo > setlocale() before returning from the newly added fix in ECPGget_desc()? > > > Regards, > Nishant Sharma, > EDB, Pune. > https://www.enterprisedb.com/ > > On Thu, Apr 9, 2026 at 4:14 PM Shruthi Gowda <gowdashru@gmail.com> wrote: > >> >> >> 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 >> > Thanks for the review Nishant. I have updated the test case patch to address comments 1–5. Regarding points 6–8, please see my detailed responses below: 6. "if (stmt.connection == NULL)" --> "if (!stmt.connection)"? --> Both formats are used interchangeably in the ecpg code. I’d prefer to stick with the explicit null check here 7. Should we return -1 in else of new fix added in ecpg_freeStmtCacheEntry()? If 'con' is NULL, why clean up the cache? Is returning -1 more defensive than cleaning the cache? --> The cache entry cleanup must happen regardless of whether the connection exists, because: - The cache slot is being reclaimed for a new statement - Memory must be freed to avoid leaks - A disconnected connection means the entry is stale and must be cleaned The NULL check protects against crashes while still performing necessary cleanup. 8. Do we need to clean up "stmt.oldlocale = ecpg_strdup(...)" and undo setlocale() before returning from the newly added fix in ECPGget_desc()? Great point! To avoid this cleanup complexity, I moved the connection check before the locale setup so the early return happens before any resources are allocated. Thanks & Regards, Shruthi K C EnterpriseDB: http://www.enterprisedb.com -
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Nishant Sharma <nishant.sharma@enterprisedb.com> — 2026-04-16T07:04:19Z
Thanks Shruthi for the new patches! I can apply v5 on all active branches (i.e master, REL_18, REL_17, REL_16, REL_15, REL_14). I checked PG's ECPG regression with v5 and v2_test on master, REL_18, REL_17, and REL_16 using both make and meson. The v5 patch looks good to me now. I only have a few observations on v2_test patch. Review comments on v2_test: 1. Not able to apply the v2 test patch on PG15 and PG14 branch, appears meson.build for test is not there on these branches. Need to remove all meson related changes for these branches. 2. char val2[5] = "data1"; --> val2's size will not be able to fit '\0' for the string. As size of val2 and number of characters in "data1" are exactly same? 3. "exec sql disconnect;" - Do we need this at the end as we have already disconnected before Test 2. 3. Minor - Do we need "printf("sqlca.sqlcode = %ld\n", sqlca.sqlcode);"? I see all others have it, but not Test 4? Regards, Nishant Sharma, Pune, EDB. https://www.enterprisedb.com/ > -
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Shruthi Gowda <gowdashru@gmail.com> — 2026-04-18T16:20:37Z
On Thu, Apr 16, 2026 at 12:34 PM Nishant Sharma < nishant.sharma@enterprisedb.com> wrote: > Thanks Shruthi for the new patches! > > I can apply v5 on all active branches (i.e master, REL_18, REL_17, REL_16, > REL_15, REL_14). > I checked PG's ECPG regression with v5 and v2_test on master, REL_18, > REL_17, and REL_16 using both make and meson. > > The v5 patch looks good to me now. I only have a few observations on > v2_test patch. > > Review comments on v2_test: > 1. Not able to apply the v2 test patch on PG15 and PG14 branch, appears > meson.build for test is not there on these branches. Need to remove all > meson related changes for these branches. > 2. char val2[5] = "data1"; --> val2's size will not be able to fit '\0' > for the string. As size of val2 and number of characters in "data1" are > exactly same? > 3. "exec sql disconnect;" - Do we need this at the end as we have already > disconnected before Test 2. > 3. Minor - Do we need "printf("sqlca.sqlcode = %ld\n", sqlca.sqlcode);"? I > see all others have it, but not Test 4? > > > Regards, > Nishant Sharma, > Pune, EDB. > https://www.enterprisedb.com/ > Thanks Nishant for the review on the latest patch. I have taken care of all the review comments for the test patch. Please find the latest test patches for master to REL_16 (v3_test) and V15 to v14 (v3_test_v15) respectively. Thanks & Regards, Shruthi K C EnterpriseDB: http://www.enterprisedb.com -
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Nishant Sharma <nishant.sharma@enterprisedb.com> — 2026-04-20T06:01:10Z
Thanks Shruthi for the new test patches! I can apply v3_test_v15 on both REL_15 and REL_14. I only have the following concern in the test patches now:- 1. The test1 table in the test case is not being cleaned up. Regards, Nishant Sharma, EDB, Pune. https://www.enterprisedb.com/
-
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Shruthi Gowda <gowdashru@gmail.com> — 2026-04-21T07:09:17Z
On Mon, Apr 20, 2026 at 11:31 AM Nishant Sharma < nishant.sharma@enterprisedb.com> wrote: > Thanks Shruthi for the new test patches! > > I can apply v3_test_v15 on both REL_15 and REL_14. > > I only have the following concern in the test patches now:- > 1. The test1 table in the test case is not being cleaned up. > > > Regards, > Nishant Sharma, > EDB, Pune. > https://www.enterprisedb.com/ > Hi Nishant, I have updated the test patches by adding a ROLLBACK command before DISCONNECT. Please find the latest test patches for master through REL_16 (v4_test) and REL_15 through REL_14 (v4_test_v15). I am also reattaching the v5 source code patch, which is applicable to all active branches (master, REL_18, REL_17, REL_16, REL_15, and REL_14).
-
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Nishant Sharma <nishant.sharma@enterprisedb.com> — 2026-04-22T04:27:00Z
Thanks Shruthi! v5 code, v4_test and v4_test_15 patches look good to me. I checked ECPG regression on master, REL_18, REL_17, REL_16, REL_15, REL_14 using both make and meson. I have finished my review work on the patches. Thank you! Regards, Nishant Sharma, EDB, Pune. https://www.enterprisedb.com/
-
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Andrew Dunstan <andrew@dunslane.net> — 2026-05-01T19:20:40Z
On Wed, Apr 22, 2026 at 12:27 AM Nishant Sharma < nishant.sharma@enterprisedb.com> wrote: > Thanks Shruthi! > > v5 code, v4_test and v4_test_15 patches look good to me. > > I checked ECPG regression on master, REL_18, REL_17, REL_16, REL_15, > REL_14 using both make and meson. > > I have finished my review work on the patches. Thank you! > > > > Thanks, everybody, pushed (as combined patches) cheers andrew
-
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Alexander Lakhin <exclusion@gmail.com> — 2026-05-05T20:00:00Z
Hello hackers, 01.05.2026 22:20, Andrew Dunstan wrote: > > On Wed, Apr 22, 2026 at 12:27 AM Nishant Sharma <nishant.sharma@enterprisedb.com> wrote: > > Thanks Shruthi! > > v5 code, v4_test and v4_test_15 patches look good to me. > > I checked ECPG regression on master, REL_18, REL_17, REL_16, REL_15, REL_14 using both make and meson. > > I have finished my review work on the patches. Thank you! > > > Thanks, everybody, pushed (as combined patches) Despite this improvement committed, dikkop keeps producing segfaults during ecpg test, e.g., [1], [2]: ok 62 - thread/thread_implicit 224 ms not ok 63 - thread/prep 116 ms # (test process was terminated by signal 11: Segmentation fault) ok 64 - thread/alloc 406 ms There is no other useful information in the log, so it's not clear what's wrong with that animal (no other gives us such failures), but I could produce something similar (on FreeBSD and Linux) with: echo "max_connections = 10" >/tmp/temp.config; TEMP_CONFIG=/tmp/temp.config gmake -s check -C src/interfaces/ecpg/test not ok 64 - thread/prep 29 ms # (test process was terminated by signal 11: Segmentation fault) not ok 65 - thread/alloc 27 ms # (test process was terminated by signal 11: Segmentation fault) gdb src/interfaces/ecpg/test/thread/prep src/interfaces/ecpg/test/core.3371028 Core was generated by `.../src/interfaces/ecpg/test/thread/prep'. Program terminated with signal SIGSEGV, Segmentation fault. #0 0x00007478ad3a8301 in deallocate_one (lineno=lineno@entry=45, c=c@entry=ECPG_COMPAT_PGSQL, con=con@entry=0x747888000ca0, prev=0x0, this=0x74788800ad40) at prepare.c:313 313 this->stmt->connection->connection, [Current thread is 1 (Thread 0x7478a1c006c0 (LWP 3371041))] (gdb) bt #0 0x00007478ad3a8301 in deallocate_one (lineno=lineno@entry=45, c=c@entry=ECPG_COMPAT_PGSQL, con=con@entry=0x747888000ca0, prev=0x0, this=0x74788800ad40) at prepare.c:313 #1 0x00007478ad3a8a32 in ECPGprepare (lineno=lineno@entry=45, connection_name=connection_name@entry=0x0, questionmarks=questionmarks@entry=false, name=name@entry=0x5d934a41b024 "i", variable=variable@entry=0x7478a1bffdb0 "INSERT INTO T VALUES ( ? )") at prepare.c:264 #2 0x00005d934a41a536 in fn (arg=<optimized out>) at .../src/interfaces/ecpg/test/thread/prep.pgc:45 #3 0x00007478ad09caa4 in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:447 #4 0x00007478ad129c3c in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:78 (gdb) p this->stmt $1 = (struct statement *) 0x242028205345554c (gdb) p this->stmt->connection Cannot access memory at address 0x2420282053455564 gdb src/interfaces/ecpg/test/thread/alloc src/interfaces/ecpg/test/core.3371068 Core was generated by `.../src/interfaces/ecpg/test/thread/alloc'. Program terminated with signal SIGSEGV, Segmentation fault. #0 pqRowProcessor (conn=conn@entry=0x7962f4000d60, errmsgp=errmsgp@entry=0x7963151ffbd0) at fe-exec.c:1226 1226 int nfields = res->numAttributes; [Current thread is 1 (Thread 0x7963152006c0 (LWP 3371075))] (gdb) bt #0 pqRowProcessor (conn=conn@entry=0x7962f4000d60, errmsgp=errmsgp@entry=0x7963151ffbd0) at fe-exec.c:1226 #1 0x00007963188e9d44 in getAnotherTuple (conn=conn@entry=0x7962f4000d60, msgLength=14) at fe-protocol3.c:849 #2 0x00007963188eb42b in pqParseInput3 (conn=conn@entry=0x7962f4000d60) at fe-protocol3.c:388 #3 0x00007963188e0e69 in parseInput (conn=conn@entry=0x7962f4000d60) at fe-exec.c:2039 #4 0x00007963188e3d74 in PQgetResult (conn=conn@entry=0x7962f4000d60) at fe-exec.c:2125 #5 0x00007963188e3fec in PQexecStart (conn=conn@entry=0x7962f4000d60) at fe-exec.c:2386 #6 0x00007963188e40a7 in PQexec (conn=0x7962f4000d60, query=0x7962e8000ca0 "select relname from pg_class where relname = 'pg_class'") at fe-exec.c:2281 #7 0x0000796318948620 in ecpg_execute (stmt=0x7962e8009e60) at execute.c:1619 #8 0x00007963189494fc in ecpg_do (lineno=<optimized out>, compat=<optimized out>, force_indicator=<optimized out>, connection_name=<optimized out>, questionmarks=questionmarks@entry=false, st=<optimized out>, query=0x5a5947b97028 "select relname from pg_class where relname = 'pg_class'", args=0x7963151ffcf0) at execute.c:2273 #9 0x00007963189495b7 in ECPGdo (lineno=lineno@entry=45, compat=compat@entry=0, force_indicator=force_indicator@entry=1, connection_name=connection_name@entry=0x0, questionmarks=questionmarks@entry=false, st=st@entry=0, query=0x5a5947b97028 "select relname from pg_class where relname = 'pg_class'") at execute.c:2298 #10 0x00005a5947b963b8 in fn (arg=<optimized out>) at .../src/interfaces/ecpg/test/thread/alloc.pgc:45 #11 0x000079631869caa4 in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:447 #12 0x0000796318729c3c in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:78 (gdb) p conn $1 = (PGconn *) 0x7962f4000d60 (gdb) p conn->result $2 = (PGresult *) 0x0 Could you please look if such crashes can be prevented too? [1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dikkop&dt=2026-05-04%2010%3A00%3A10 [2] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dikkop&dt=2026-05-03%2021%3A25%3A17 Best regards, Alexander
-
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Tom Lane <tgl@sss.pgh.pa.us> — 2026-05-05T20:32:41Z
Alexander Lakhin <exclusion@gmail.com> writes: > There is no other useful information in the log, so it's not clear what's > wrong with that animal (no other gives us such failures), but I could > produce something similar (on FreeBSD and Linux) with: > echo "max_connections = 10" >/tmp/temp.config; TEMP_CONFIG=/tmp/temp.config gmake -s check -C src/interfaces/ecpg/test Yes, I can also reproduce problems with the ecpg tests at max_connections = 10. For me, thread/prep segfaults but thread/alloc just seems to hang indefinitely. (thread/prep sometimes does too.) These issues are not new; v18 does the same. The reporting is a bit different but I think that's from pg_regress changes not ecpg. Looking at the postmaster log, I see 2026-05-05 16:11:06.509 EDT [682116] FATAL: sorry, too many clients already which is unsurprising in this situation, but apparently these tests don't handle a connection failure well at all. There's no such message in dikkop's log, so that may be an unrelated problem. BTW, reducing max_connections to 5 causes several other tests to fail, but in unsurprising ways, like # +SQL error: could not connect to database "ecpg1_regression" on line 107 # +SQL error: could not connect to database "ecpg1_regression" on line 107 # +SQL error: could not connect to database "ecpg1_regression" on line 107 # +SQL error: could not connect to database "ecpg1_regression" on line 107 regards, tom lane
-
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Andrew Dunstan <andrew@dunslane.net> — 2026-05-05T21:36:02Z
On 2026-05-05 Tu 4:32 PM, Tom Lane wrote: > Alexander Lakhin <exclusion@gmail.com> writes: >> There is no other useful information in the log, so it's not clear what's >> wrong with that animal (no other gives us such failures), but I could >> produce something similar (on FreeBSD and Linux) with: >> echo "max_connections = 10" >/tmp/temp.config; TEMP_CONFIG=/tmp/temp.config gmake -s check -C src/interfaces/ecpg/test > Yes, I can also reproduce problems with the ecpg tests at > max_connections = 10. For me, thread/prep segfaults but thread/alloc > just seems to hang indefinitely. (thread/prep sometimes does too.) > These issues are not new; v18 does the same. The reporting is a > bit different but I think that's from pg_regress changes not ecpg. > > Looking at the postmaster log, I see > > 2026-05-05 16:11:06.509 EDT [682116] FATAL: sorry, too many clients already > > which is unsurprising in this situation, but apparently these tests > don't handle a connection failure well at all. > > There's no such message in dikkop's log, so that may be an unrelated problem. > > BTW, reducing max_connections to 5 causes several other tests to fail, > but in unsurprising ways, like > > # +SQL error: could not connect to database "ecpg1_regression" on line 107 > # +SQL error: could not connect to database "ecpg1_regression" on line 107 > # +SQL error: could not connect to database "ecpg1_regression" on line 107 > # +SQL error: could not connect to database "ecpg1_regression" on line 107 > > > Ugh. I will do some digging. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
-
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Andrew Dunstan <andrew@dunslane.net> — 2026-05-06T11:55:10Z
On 2026-05-05 Tu 5:36 PM, Andrew Dunstan wrote: > > On 2026-05-05 Tu 4:32 PM, Tom Lane wrote: >> Alexander Lakhin <exclusion@gmail.com> writes: >>> There is no other useful information in the log, so it's not clear >>> what's >>> wrong with that animal (no other gives us such failures), but I could >>> produce something similar (on FreeBSD and Linux) with: >>> echo "max_connections = 10" >/tmp/temp.config; >>> TEMP_CONFIG=/tmp/temp.config gmake -s check -C src/interfaces/ecpg/test >> Yes, I can also reproduce problems with the ecpg tests at >> max_connections = 10. For me, thread/prep segfaults but thread/alloc >> just seems to hang indefinitely. (thread/prep sometimes does too.) >> These issues are not new; v18 does the same. The reporting is a >> bit different but I think that's from pg_regress changes not ecpg. >> >> Looking at the postmaster log, I see >> >> 2026-05-05 16:11:06.509 EDT [682116] FATAL: sorry, too many clients >> already >> >> which is unsurprising in this situation, but apparently these tests >> don't handle a connection failure well at all. >> >> There's no such message in dikkop's log, so that may be an unrelated >> problem. >> >> BTW, reducing max_connections to 5 causes several other tests to fail, >> but in unsurprising ways, like >> >> # +SQL error: could not connect to database "ecpg1_regression" on >> line 107 >> # +SQL error: could not connect to database "ecpg1_regression" on >> line 107 >> # +SQL error: could not connect to database "ecpg1_regression" on >> line 107 >> # +SQL error: could not connect to database "ecpg1_regression" on >> line 107 >> >> >> > > > Ugh. I will do some digging. > > > OK, first this is orthogonal to the issue fixed earlier in this thread. It's a 22 yer old bug where a connection failure results in a thread falling back to a sibling's connection. The fix is to keep track of which thread opened which connection and only fall back to the global actual_connection if it was started by our thread. There was an unresolved report of these symptoms in 2006[1]. Essentially, the user was holding the lock the docs told him to hold, and ecpglib still corrupted state because the corruption window was inside ecpglib, not inside the libpq calls he was trying to serialize. It's not an easy problem to diagnose, however, so there could well have been more cases. Attached is a patch with the fix, courtesy of claude. It's a slight behaviour change: After the patch, programs in this category fail loudly with ECPG_NO_CONN: - Pattern that breaks: main thread calls EXEC SQL CONNECT, spawns workers, workers issue EXEC SQL ... with no connection name and no per-thread SET CONNECTION, relying on their own mutex to serialize the libpq calls. - Migration: one of two existing supported patterns. Either name the connection explicitly per statement (EXEC SQL AT con1 SELECT ...) or set the per-thread default once at thread start (EXEC SQL SET CONNECTION con1;). The latter still works under the patch because it explicitly populates the per-thread slot, and the patch only owner-checks the global fallback, not the per-thread slot. So I guess the question is whether or not we backpatch it (or some other fix)? cheers andrew [1] https://www.postgresql.org/message-id/52940eef0611100514p14c85e34l528da656662decc9@mail.gmail.com -- Andrew Dunstan EDB: https://www.enterprisedb.com
-
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Tom Lane <tgl@sss.pgh.pa.us> — 2026-05-06T14:58:27Z
Andrew Dunstan <andrew@dunslane.net> writes: > Attached is a patch with the fix, courtesy of claude. It's a slight > behaviour change: Yeah. So we could either do something like this, or say that the test case is buggy and needs to provide its own mutexes, per the existing comment - * if no connection in TSD for this thread, get the global default - * connection and hope the user knows what they're doing (i.e. using - * their own mutex to protect that connection from concurrent accesses On the whole I think I favor the behavior change. We might get some complaints, but it just seems a lot safer to redefine it like this. Either way, it seems like some documentation adjustments are called for. As far as the patch itself goes, I'd be inclined to pull the preparatory step ecpg_pthreads_init(); /* ensure actual_connection_key is valid */ into the new ecpg_default_connection() subroutine, especially since its proposed comment doesn't mention that prerequisite. regards, tom lane -
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Andrew Dunstan <andrew@dunslane.net> — 2026-05-06T15:50:04Z
On 2026-05-06 We 10:58 AM, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: >> Attached is a patch with the fix, courtesy of claude. It's a slight >> behaviour change: > Yeah. So we could either do something like this, or say that the > test case is buggy and needs to provide its own mutexes, per the > existing comment > > - * if no connection in TSD for this thread, get the global default > - * connection and hope the user knows what they're doing (i.e. using > - * their own mutex to protect that connection from concurrent accesses > > On the whole I think I favor the behavior change. We might get some > complaints, but it just seems a lot safer to redefine it like this. I agree. > > Either way, it seems like some documentation adjustments are called > for. > > As far as the patch itself goes, I'd be inclined to pull the > preparatory step > > ecpg_pthreads_init(); /* ensure actual_connection_key is valid */ > > into the new ecpg_default_connection() subroutine, especially since > its proposed comment doesn't mention that prerequisite. > > Right. Given the lack of field complaints, maybe we should wait until after next week's releases? cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
-
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Tom Lane <tgl@sss.pgh.pa.us> — 2026-05-06T16:02:08Z
Andrew Dunstan <andrew@dunslane.net> writes: > Given the lack of field complaints, maybe we should wait until after > next week's releases? Yeah, given that this bug has gone almost undetected for decades, I don't think there's a rush. Let's sit on it another week. regards, tom lane
-
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com> — 2026-05-23T18:59:44Z
Hi On Fri, May 1, 2026 at 12:21 PM Andrew Dunstan <andrew@dunslane.net> wrote: > > > On Wed, Apr 22, 2026 at 12:27 AM Nishant Sharma < > nishant.sharma@enterprisedb.com> wrote: > >> Thanks Shruthi! >> >> v5 code, v4_test and v4_test_15 patches look good to me. >> >> I checked ECPG regression on master, REL_18, REL_17, REL_16, REL_15, >> REL_14 using both make and meson. >> >> I have finished my review work on the patches. Thank you! >> >> >> >> > Thanks, everybody, pushed (as combined patches) > Looks like this committed a case, attached a patch to fix this. Thanks, Satya
-
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Shruthi Gowda <gowdashru@gmail.com> — 2026-05-25T07:46:20Z
On Sun, May 24, 2026 at 12:29 AM SATYANARAYANA NARLAPURAM < satyanarlapuram@gmail.com> wrote: > Hi > > On Fri, May 1, 2026 at 12:21 PM Andrew Dunstan <andrew@dunslane.net> > wrote: > >> >> >> On Wed, Apr 22, 2026 at 12:27 AM Nishant Sharma < >> nishant.sharma@enterprisedb.com> wrote: >> >>> Thanks Shruthi! >>> >>> v5 code, v4_test and v4_test_15 patches look good to me. >>> >>> I checked ECPG regression on master, REL_18, REL_17, REL_16, REL_15, >>> REL_14 using both make and meson. >>> >>> I have finished my review work on the patches. Thank you! >>> >>> >>> >>> >> Thanks, everybody, pushed (as combined patches) >> > > Looks like this committed a case, attached a patch to fix this. > > Thanks, > Satya > > Thanks Satya for looking into this. However, we don't need to add the validation here because the caller ecpg_do_prologue*()* has already taken care of it. Adding it again would be redundant. What do you think? Thanks & Regards, Shruthi K C EnterpriseDB: http://www.enterprisedb.com
-
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Michael Paquier <michael@paquier.xyz> — 2026-05-25T23:34:43Z
On Mon, May 25, 2026 at 01:16:20PM +0530, Shruthi Gowda wrote: > Thanks Satya for looking into this. However, we don't need to add the > validation here because the caller ecpg_do_prologue*()* has already taken > care of it. Adding it again would be redundant. What do you think? Right, but I think that this points at a different issue: it is wasteful to call a second time ecpg_get_connection() to get a connection while ecpg_do_prologue() has already done a cache lookup for the connection. Wouldn't it be better to pass directly the connection obtained by the first call of ecpg_get_connection() as a function argument of ecpg_auto_prepare(), replacing the connection name? If we need the extra init() at the end, please show if this can go wrong in the shape of a regression test. -- Michael
-
Re: [BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL
Shruthi Gowda <gowdashru@gmail.com> — 2026-05-26T11:40:02Z
On Tue, May 26, 2026 at 5:04 AM Michael Paquier <michael@paquier.xyz> wrote: > On Mon, May 25, 2026 at 01:16:20PM +0530, Shruthi Gowda wrote: > > Thanks Satya for looking into this. However, we don't need to add the > > validation here because the caller ecpg_do_prologue*()* has already taken > > care of it. Adding it again would be redundant. What do you think? > > Right, but I think that this points at a different issue: it is > wasteful to call a second time ecpg_get_connection() to get a > connection while ecpg_do_prologue() has already done a cache lookup > for the connection. > > Wouldn't it be better to pass directly the connection obtained by the > first call of ecpg_get_connection() as a function argument of > ecpg_auto_prepare(), replacing the connection name? If we need the > extra init() at the end, please show if this can go wrong in the shape > of a regression test. > -- > Michael > I agree. We can avoid the duplicate call to ecpg_get_connection() by passing the connection object directly to ecpg_auto_prepare(). However, we still need to pass the connection name as well, because underlying functions inside ecpg_auto_prepare()—such as ECPGprepare() and AddStmtToCache()— expect the connection name as an argument. Furthermore, ECPGprepare() corresponds directly to the embedded SQL EXEC SQL PREPARE statement, meaning we cannot entirely avoid calling ecpg_get_connection() within that flow anyway. Thanks & Regards, Shruthi K C EnterpriseDB: http://www.enterprisedb.com