To: vim_dev@googlegroups.com Subject: Patch 8.1.0234 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.1.0234 Problem: Incorrect reference counting in Perl interface. Solution: Call SvREFCNT_inc more often, add a test. (Damien) Files: src/if_perl.xs, src/testdir/test_perl.vim *** ../vim-8.1.0233/src/if_perl.xs 2018-07-25 22:02:32.231966301 +0200 --- src/if_perl.xs 2018-08-02 21:43:01.012805454 +0200 *************** *** 831,838 **** ptr->w_perl_private = newSV(0); sv_setiv(ptr->w_perl_private, PTR2IV(ptr)); } ! else ! SvREFCNT_inc_void_NN(ptr->w_perl_private); SvRV(rv) = ptr->w_perl_private; SvROK_on(rv); return sv_bless(rv, gv_stashpv("VIWIN", TRUE)); --- 831,837 ---- ptr->w_perl_private = newSV(0); sv_setiv(ptr->w_perl_private, PTR2IV(ptr)); } ! SvREFCNT_inc_void_NN(ptr->w_perl_private); SvRV(rv) = ptr->w_perl_private; SvROK_on(rv); return sv_bless(rv, gv_stashpv("VIWIN", TRUE)); *************** *** 847,854 **** ptr->b_perl_private = newSV(0); sv_setiv(ptr->b_perl_private, PTR2IV(ptr)); } ! else ! SvREFCNT_inc_void_NN(ptr->b_perl_private); SvRV(rv) = ptr->b_perl_private; SvROK_on(rv); return sv_bless(rv, gv_stashpv("VIBUF", TRUE)); --- 846,852 ---- ptr->b_perl_private = newSV(0); sv_setiv(ptr->b_perl_private, PTR2IV(ptr)); } ! SvREFCNT_inc_void_NN(ptr->b_perl_private); SvRV(rv) = ptr->b_perl_private; SvROK_on(rv); return sv_bless(rv, gv_stashpv("VIBUF", TRUE)); *************** *** 918,929 **** else rv = newBUFrv(newSV(0), curbuf); ! if (SvRV(sv) == SvRV(rv)) ! SvREFCNT_dec(SvRV(rv)); ! else // XXX: Not sure if the `else` condition are right ! // Test_SvREFCNT() pass in all case. sv_setsv(sv, rv); return 0; } #endif /* !PROTO */ --- 916,928 ---- else rv = newBUFrv(newSV(0), curbuf); ! if (SvRV(sv) != SvRV(rv)) ! // XXX: This magic variable is a bit confusing... ! // Is curently refcounted ? sv_setsv(sv, rv); + SvREFCNT_dec(rv); + return 0; } #endif /* !PROTO */ *** ../vim-8.1.0233/src/testdir/test_perl.vim 2018-07-25 22:02:32.235966277 +0200 --- src/testdir/test_perl.vim 2018-08-02 21:39:46.757852657 +0200 *************** *** 4,9 **** --- 4,12 ---- finish end + " FIXME: RunTest don't see any error when Perl abort... + perl $SIG{__WARN__} = sub { die "Unexpected warnings from perl: @_" }; + func Test_change_buffer() call setline(line('$'), ['1 line 1']) perl VIM::DoCommand("normal /^1\n") *************** *** 229,234 **** --- 232,246 ---- #line 5 "Test_000_SvREFCNT()" my ($b, $w); + my $num = 0; + for ( 0 .. 100 ) { + if ( ++$num >= 8 ) { $num = 0 } + VIM::DoCommand("buffer X$num"); + $b = $curbuf; + } + + VIM::DoCommand("buffer t"); + $b = $curbuf for 0 .. 100; $w = $curwin for 0 .. 100; () = VIM::Buffers for 0 .. 100; *************** *** 240,251 **** my $cw = Internals::SvREFCNT($$w); VIM::Eval("assert_equal(2, $cb, 'T1')"); VIM::Eval("assert_equal(2, $cw, 'T2')"); foreach ( VIM::Buffers, VIM::Windows ) { my $c = Internals::SvREFCNT($_); VIM::Eval("assert_equal(2, $c, 'T3')"); $c = Internals::SvREFCNT($$_); ! # Why only one ref? ! # Look wrong but work. Maybe not portable... VIM::Eval("assert_equal(1, $c, 'T4')"); } $cb = Internals::SvREFCNT($$curbuf); --- 252,264 ---- my $cw = Internals::SvREFCNT($$w); VIM::Eval("assert_equal(2, $cb, 'T1')"); VIM::Eval("assert_equal(2, $cw, 'T2')"); + my $strongref; foreach ( VIM::Buffers, VIM::Windows ) { + VIM::DoCommand("%bw!"); my $c = Internals::SvREFCNT($_); VIM::Eval("assert_equal(2, $c, 'T3')"); $c = Internals::SvREFCNT($$_); ! next if $c == 2 && !$strongref++; VIM::Eval("assert_equal(1, $c, 'T4')"); } $cb = Internals::SvREFCNT($$curbuf); *** ../vim-8.1.0233/src/version.c 2018-08-01 19:05:59.286223185 +0200 --- src/version.c 2018-08-02 21:44:31.440307634 +0200 *************** *** 796,797 **** --- 796,799 ---- { /* Add new patch number below this line */ + /**/ + 234, /**/ -- I'm sure that I asked CBuilder to do a "full" install. Looks like I got a "fool" install, instead. Charles E Campbell, Jr, PhD /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///