Skip to content

Commit 3a16cb5

Browse files
committed
doio: fix shmread() on non-string buffers
- If the buffer is a reference, don't leak memory (or abort, on debugging perls). The problem is that SvPOK_only() blindly turns off some SV flags, but does not decrement any refcounts if ROK was on. - If the buffer is tied, don't call FETCH. Conceptually, shmread() is a bytestring assignment (from a shared memory segment to a scalar variable), so it should only STORE. (This is also why most of the code can be replaced by sv_setpvn().) Fixes #22898.
1 parent 3caa23e commit 3a16cb5

File tree

3 files changed

+25
-13
lines changed

3 files changed

+25
-13
lines changed

doio.c

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3421,17 +3421,8 @@ Perl_do_shmio(pTHX_ I32 optype, SV **mark, SV **sp)
34213421
return -1;
34223422

34233423
if (optype == OP_SHMREAD) {
3424-
SvGETMAGIC(mstr);
3425-
SvUPGRADE(mstr, SVt_PV);
3426-
/* suppress warning when reading into undef var (tchrist 3/Mar/00) */
3427-
if (! SvOK(mstr))
3428-
SvPVCLEAR(mstr);
3429-
SvPOK_only(mstr);
3430-
char *const mbuf = SvGROW(mstr, (STRLEN)msize+1);
3431-
3432-
Copy(shm + mpos, mbuf, msize, char);
3433-
SvCUR_set(mstr, msize);
3434-
*SvEND(mstr) = '\0';
3424+
sv_setpvn(mstr, shm + mpos, msize);
3425+
SvUTF8_off(mstr);
34353426
SvSETMAGIC(mstr);
34363427
/* who knows who has been playing with this shared memory? */
34373428
SvTAINTED_on(mstr);

pod/perldelta.pod

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,14 @@ XXX
393393
L<perlfunc/shmread> and L<perlfunc/shmwrite> are no longer limited to 31-bit
394394
values for their POS and SIZE arguments. [GH #22895]
395395

396+
=item *
397+
398+
L<perlfunc/shmread> is now better behaved if VAR is not a plain string. If VAR
399+
is a tied variable, it calls C<STORE> once; previously, it would also call
400+
C<FETCH>, but without using the result. If VAR is a reference, the referenced
401+
entity has its refcount properly decremented when VAR is turned into a string;
402+
previously, it would leak memory. [GH #22898]
403+
396404
=back
397405

398406
=head1 Known Problems

t/io/shm.t

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ if (not defined $key) {
5454
}
5555
}
5656
else {
57-
plan(tests => 28);
57+
plan(tests => 33);
5858
pass('acquired shared mem');
5959
}
6060

@@ -87,7 +87,7 @@ my ($fetch, $store) = (0, 0);
8787
sub STORE { ++$store; $_[0][0] = $_[1] } }
8888
tie my $ct, 'Counted';
8989
shmread $key, $ct, 0, 1;
90-
is($fetch, 1, "shmread FETCH once");
90+
is($fetch, 0, "shmread FETCH none");
9191
is($store, 1, "shmread STORE once");
9292
($fetch, $store) = (0, 0);
9393
shmwrite $key, $ct, 0, 1;
@@ -110,6 +110,19 @@ is($store, 0, "shmwrite STORE none");
110110
is($rdbuf, $text, "check we got back the expected (upgraded source)");
111111
}
112112

113+
# GH #22898 - reading into reference is sane
114+
{
115+
my $rdbuf = [];
116+
builtin::weaken(my $wref = $rdbuf);
117+
118+
my $text = 'A';
119+
ok(shmwrite($key, $text, 0, 1), "wrote 'A' to shared segment");
120+
ok(shmread($key, $rdbuf, 0, 1), "read 1 byte into buffer that previously stored a ref");
121+
is(ref($rdbuf), '', "buffer is not a reference anymore");
122+
is($rdbuf, $text, "buffer contains expected string");
123+
is($wref, undef, "no leak: referenced object had refcount decremented");
124+
}
125+
113126
# GH #22895 - 2^31 boundary
114127
SKIP: {
115128
skip("need at least 5GB of memory for this test", 5)

0 commit comments

Comments
 (0)