Release v1.95#204
Conversation
There was a problem hiding this comment.
Pull request overview
This release (v1.91) includes improvements to test reliability, security enhancements, and CI infrastructure updates for the DBD-Oracle Perl database driver.
Changes:
- Enhanced test portability by adding signal fallback mechanisms for systems without USR1/USR2 support
- Improved security by replacing unsafe sprintf calls with snprintf in C code
- Updated CI configuration to use Oracle Free instead of Oracle XE
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| t/92-segv-fork.t | Added documentation about SEGV issue, new thread count checking function, and reduced test job count for faster execution |
| t/92-segv-fork.pl | Reduced random sleep time range to speed up tests |
| t/91-segv-fork.t | Added signal portability with USR1/USR2 fallback to HUP/INT for OS compatibility |
| oci8.c | Security improvement: replaced sprintf with snprintf and increased buffer sizes |
| .github/workflows/action.yml | Updated CI to use Oracle Free instead of Oracle XE |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #!/usr/bin/env perl | ||
|
|
||
| ## A SEGV during this test is a sign that Perl itself lacks the patch | ||
| ## that allows a SIGCHLD (any interrupt) to be handedled using a worker |
There was a problem hiding this comment.
Typo in comment: "handedled" should be "handled".
| ## that allows a SIGCHLD (any interrupt) to be handedled using a worker | |
| ## that allows a SIGCHLD (any interrupt) to be handled using a worker |
| note Dumper( $dbh->selectall_arrayref(qq|SELECT SYSTIMESTAMP AT TIME ZONE 'UTC' FROM DUAL|)); | ||
| } | ||
| $dbh = undef; | ||
| # Not important but an indication SEGV is eminent |
There was a problem hiding this comment.
Typo in comment: "eminent" should be "imminent". "Eminent" means distinguished or prominent, while "imminent" means about to happen, which is the intended meaning here.
| # Not important but an indication SEGV is eminent | |
| # Not important but an indication SEGV is imminent |
| } | ||
| $dbh = undef; | ||
| # Not important but an indication SEGV is eminent | ||
| # (wont't PASS if Perl is not built with threads support) |
There was a problem hiding this comment.
Typo in comment: "wont't" should be "won't" (incorrect apostrophe placement).
| # (wont't PASS if Perl is not built with threads support) | |
| # (won't PASS if Perl is not built with threads support) |
| # printf "# [ %s ]\n", $row->[]; | ||
|
|
||
| my $usleep = int(rand(300000)) + 2000000; # 2-5 seconds | ||
| my $usleep = int(rand(100000)) + 2000000; # 1-3 seconds (to speed up test!) |
There was a problem hiding this comment.
The comment states "1-3 seconds" but the code calculates int(rand(100000)) + 2000000, which produces values between 2,000,000 and 2,099,999 microseconds. This corresponds to approximately 2.0-2.1 seconds, not 1-3 seconds. The comment should be corrected to accurately reflect the actual range.
| my $usleep = int(rand(100000)) + 2000000; # 1-3 seconds (to speed up test!) | |
| my $usleep = int(rand(100000)) + 2000000; # about 2.0-2.1 seconds (to speed up test!) |
|
I have tagged v1.91_3 and will shortly arrange to have it pushed to the cpan |
|
1.91_3 is now on the cpan https://metacpan.org/release/ZARQUON/DBD-Oracle-1.91_3 |
|
t/92-segv-fork.t hangs on Debian unstable sometimes, and when it doesn't it fails: |
6f56db5 to
abe105c
Compare
303ade6 to
ac54b4d
Compare
|
I have tagged 1.91_5 and its being released to the cpan. Lots of changes in that. I suspect it will be the last. |
|
To give it some distance lets jump to 1.95 1.90->1.91 implies less distance between releases than there is. |
|
I think i will ship this tonight. |
and someting is marginal. I didn't see the warnings and unable to locate a test that triggerd the use of this code path.
least one additional non-Perl thread is created. If this thread is tapped to handle the SIGCHLD SEGV will result.
Allow test not to fail on mistake
…e GitHub test does not have thread support built in.
USR1 & USR2; replaced with HUP and INT for OS's (Windows) Confirmed that the Perl Interrupt patch and it's alternative work suitably well at protecting against the SEGV caused by Oracle instant client non-Perl worker threads.
On Perl < 5.19.1 (before copy-on-write became default), the test
t/25plsql.t line 232 ('expected return length') fails because
length($p1) returns 5 instead of 200.
Root cause: when an SV is undef'd after holding a value (e.g. 'null!'
with SvCUR=5), SvCUR retains the stale length. SvGROW does not reset
SvCUR. When the inout buffer is grown to exactly match maxlen (200),
SvLEN also becomes 200. After Oracle writes 200 bytes (phs->alen=200),
the UNTOUCHED heuristic in dbd_phs_sv_complete() sees phs->alen ==
SvLEN(sv) and falsely concludes Oracle didn't touch the buffer, falling
back to SvCUR(sv) which is the stale value 5.
Perl 5.19.1+ masked this because COW changes how SvGROW allocates
buffers, making SvLEN differ from the requested size and avoiding
the false positive in the heuristic.
Two fixes applied:
1. dbd_rebind_ph_char(): After SvGROW for inout params, reset SvCUR
to 0 when the SV is undef. This eliminates stale SvCUR values.
2. dbd_phs_sv_complete(): Add SvPOK(sv) guard to the UNTOUCHED
heuristic. When an SV was undef (SvPOK off) before execute and
Oracle reports indp==0, Oracle definitively wrote data, so trust
phs->alen. The UNTOUCHED fallback now only applies when the SV
had valid string content before execute.
Fixes: #216
_SIG_CHLD called waitpid(-1, WNOHANG) exactly once per signal delivery. POSIX standard signals coalesce: if two or more children exit between Perl safe-signal opchecks, only a single SIGCHLD is queued. The handler would then reap one child while the others remained as zombies in $WORKSET, so running() never reached 0 and the while(isBusy) loop in FORK_SEGV span forever. Three changes: 1. Loop waitpid in _SIG_CHLD until no more children are available, draining all zombies per signal delivery. 2. Add a defensive reap() method called from the main polling loop, so exited children are collected even if a signal was lost or coalesced between opchecks. 3. Add a 300-second deadline to the main loop; call abort() instead of hanging indefinitely. The same underlying bug (signal coalescing + single waitpid) was also the likely cause of the Windows/Strawberry Perl hangs reported in gh#191, although that issue has additional problems with missing USR1/USR2 signals on Windows. Fixes: gh#212 See-also: gh#191
See gh#212 for full details. But there is a bug in perl prior to this.
The join() call was inside the foreach loop, wastefully re-joining on every iteration. It produced correct results only by accident on the final iteration. Move it after the loop so all elements are quoted before joining. Also simplify the quoting expression using string interpolation.
ora_server_version() computed the version from v$version and cached
it in $dbh->{ora_server_version}, but fell off the end of the sub
without returning the value. Only the second call (which hit the
cache) returned correctly.
All four internal callers dereference the result as an arrayref
(e.g. ora_server_version($dbh)->[0]), so the first call on any
connection where the XS layer had not already populated the cache
would die with "Can't use an undefined value as an ARRAY reference".
Add the missing return statement, replacing the TODO comment that
already identified the bug.
Fixes: gh#205
The SQL uses :errormsg but the bind used :proc, a copy-paste error from dbms_msgpipe_get. This caused dbms_msgpipe_ack to always fail at runtime since :proc doesn't exist in the acknowledge statement. Remove debug line
The original rt91217 fix (v1.71_00) only worked in statistics_info. In column_info, an elsif chained to if (>= 8) made the >= 11 branch unreachable, so the hint was never removed. In table_info, primary_key_info, and foreign_key_info the hint was hardcoded in single-quoted heredocs and never addressed at all. - column_info: change elsif to standalone if so hint is cleared for 11+ - table_info, primary_key_info, foreign_key_info: remove hardcoded hint
## Overview
Address test hangs reported in gh#215 (and related gh#191, gh#212) where
`t/90-segv-threads.t` and `t/91-segv-fork.t` hang or crash on Windows
and older Perls. All three segv tests now run only on Linux, with
additional guards for minimum dependency versions.
## Changes
- **t/90-segv-threads.t**
- Add Linux-only skip guard (`$^O ne 'linux'`)
- Require `Thread::Queue 3.07` to avoid objects arriving as unblessed references across thread boundaries on older versions
- Add 30-second timeout to thread join loop in `disable()` to prevent indefinite hangs when threads terminate abnormally
- **t/91-segv-fork.t**
- Add Linux-only skip guard (`$^O ne 'linux'`)
- Fix bug in `disable()` where `kill('USR2', ...)` was hardcoded instead of using the `$SIG_EXIT` variable, ignoring the signal fallback logic
- **t/92-segv-fork.t**
- Add Linux-only skip guard (prior change)
- Add Perl 5.42+ version requirement skip guard (prior change)
Oracle Database 23ai introduces a native SQL BOOLEAN type with internal
OCI type code 252 (SQLT_BOL). Previously this type fell through to the
default handler, producing a spurious warning and reporting the wrong
DBI type code (-9252) in `$sth->{TYPE}`.
Changes:
- `dbdimp.c`: Add `#ifndef SQL_BOOLEAN` fallback (value 16) for older
DBI installations. Add case 252 to `oratype_bind_ok()` so BOOLEAN
columns are accepted as bind targets. Add `SQL_BOOLEAN -> 252`
mapping in `ora_sql_type()` for DBI-standard bind type round-trip.
Add `252 -> SQL_BOOLEAN` mapping in `ora2sql_type()` so
`$sth->{TYPE}` correctly reports `SQL_BOOLEAN` (16).
- `oci8.c`: Add case 252 in `dbd_describe()` with `disize = 6` to
accommodate the longest string representation ("FALSE\0") when OCI
converts SQLT_BOL to a null-terminated string, avoiding a buffer
overflow.
- `Oracle.xs`: Export `ORA_BOOLEAN = 252` as a named constant.
- `lib/DBD/Oracle.pm`: Add `ORA_BOOLEAN` to the `:ora_types` export
tag and update all documentation sections listing Oracle type
constants. Add a description of the type and its SQL_BOOLEAN mapping.
- `t/20select.t`: Add 5 tests in a version-gated SKIP block
(requires Oracle >= 23) covering the ORA_BOOLEAN constant value,
`$sth->{TYPE}` reporting SQL_BOOLEAN, and fetched true/false values.
When a PL/SQL block returned a SYS_REFCURSOR that was never opened (OCI_STMT_STATE_INITIALIZED), pp_exec_rset() replaced phs->sv with newSV(0) without first decrementing the refcount on the original SV. This leaked the user's bound scalar on every execute and silently disconnected it from the placeholder, so subsequent re-executes of the same statement wrote into an orphaned SV instead of the user's variable. Replace the `phs->sv = newSV(0)` assignment with `SvOK_off(phs->sv)`, which clears the SV to undef in place. This preserves the original binding, avoids the refcount leak, and keeps re-execute working correctly. The OCI_STMT_STATE_INITIALIZED path was introduced for RT#82663 (null cursor handling) but the leak has been present since that change. Fixes: gh#217
Document how to connect to Oracle over TLS/SSL using TCPS protocol. TLS is handled by the Oracle client libraries (OCI) and works transparently once the client-side sqlnet.ora and wallet/certificates are configured. Covers Oracle Wallet setup, sqlnet.ora configuration, system certificates (19c+), TNS entries with TCPS, and inline connect descriptors.
- Add code examples for enabling FAN via connect attribute and environment variable - Explain what FAN does (HA events from Oracle RAC clusters) - Show common usage pattern combining `ora_events` with `ora_drcp` - Clarify that `ora_events` is **not** related to Oracle Database Change Notification (DCN/CQN) - Remove duplicate `ORA_EVENTS` environment variable check
Binding a placeholder with `ora_type => ORA_DATE` previously caused a fatal error: > Can't bind :p1, ora_type 12 not supported by DBD::Oracle `ORA_DATE` (type 12) was missing from the `oratype_bind_ok()` whitelist in `dbdimp.c`. The bound value is handled as a string by `dbd_rebind_ph_char()` (the default path), which correctly passes it to Oracle for implicit NLS date conversion — consistent with how the fetch side already handles `ORA_DATE`. - **dbdimp.c** — Added `case 12: /* DATE */` to `oratype_bind_ok()` switch - **t/95-gh84_ora_date_bind.t** — New test: verifies `bind_param` with `ora_type => ORA_DATE` no longer croaks, executes an INSERT, and round-trips the value via SELECT - **MANIFEST** — Added `t/95-gh84_ora_date_bind.t` (and two previously missing `t/94-*` entries)
Guard the call to `SvGROW` in `dbdimp.c` so we only grow the Perl SV when `phs->maxlen > 0`. Previously `SvGROW(phs->sv, phs->maxlen-1)` could underflow when `phs->maxlen` was 0 (e.g. bind_param_inout with undef), producing a huge unsigned size and triggering Perl's "memory wrap" panic (seen in GH#105 / RT94232). This adds a simple defensive check and keeps behavior identical when `maxlen` is positive. - File changed: `dbdimp.c` - Key change: only call `SvGROW(phs->sv, ...)` for `phs->ftype == 96` when `phs->maxlen > 0`. This fixes the panic observed when rebinding zero-length in/out parameters and prevents passing an enormous value to `SvGROW`.
Release v1.91