Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NullPointerException when using sub form of map on array with deleted element #5356

Closed
p6rt opened this issue Jun 4, 2016 · 12 comments
Closed
Labels
JVM Related to Rakudo-JVM

Comments

@p6rt
Copy link

p6rt commented Jun 4, 2016

Migrated from rt.perl.org#128320 (status was 'resolved')

Searchable as RT128320$

@p6rt
Copy link
Author

p6rt commented Jun 4, 2016

From @usev6

The following code (tested in S32-array/delete.t) started to fail with rakudo-j recently​:

$ perl6-j -e 'my @​a = 0..1; @​a[0]​:delete; map { 1 }, @​a'
java.lang.NullPointerException
  in block <unit> at -e line 1

$ perl6-j --ll-exception -e 'my @​a = 0..1; @​a[0]​:delete; map { 1 }, @​a'
java.lang.NullPointerException
  in add_to_cache (gen/jvm/BOOTSTRAP.nqp​:1512)
  in (gen/jvm/BOOTSTRAP.nqp​:1516)
  in push (gen/jvm/CORE.setting​:1828)
  in push-all (gen/jvm/CORE.setting​:14627)
  in push-until-lazy (gen/jvm/CORE.setting​:2401)
  in (gen/jvm/CORE.setting​:14308)
  in (gen/jvm/CORE.setting​:14306)
  in reify-until-lazy (gen/jvm/CORE.setting​:14304)
  in is-lazy (gen/jvm/CORE.setting​:14892)
  in map (gen/jvm/CORE.setting​:5366)
  in map (gen/jvm/CORE.setting​:5365)
  in <unit> (-e​:1)
  in <unit-outer> (-e​:1)
  in eval (gen/jvm/stage2/NQPHLL.nqp​:1198)
  in eval (src/Perl6/Compiler.nqp​:171)
  in (gen/jvm/stage2/NQPHLL.nqp​:1288)
  in command_eval (gen/jvm/stage2/NQPHLL.nqp​:1285)
  in command_eval (src/Perl6/Compiler.nqp​:29)
  in command_line (gen/jvm/stage2/NQPHLL.nqp​:1269)
  in MAIN (gen/jvm/main.nqp​:37)
  in <mainline> (gen/jvm/main.nqp​:33)
  in (gen/jvm/main.nqp)

I think, the error first appeared with Rakudo commit beb3c986.

The following patch makes the NPE go away (and has no spectest fallout). But I'm not sure if it is the right thing to insert Nil when nqp​::atpos returns null.

====

Inline Patch
diff --git a/src/core/List.pm b/src/core/List.pm
index 0c65a6f..ddc8607 100644
--- a/src/core/List.pm
+++ b/src/core/List.pm
@@ -476,7 +476,7 @@ my class List does Iterable does Positional { # declared in BOOTSTRAP
                 method push-all($target) {
                     my int $elems = nqp::elems($!reified);
                     my $no-sink;
-                    $no-sink := $target.push(nqp::atpos($!reified,$!i))
+                    $no-sink := $target.push(nqp::ifnull(nqp::atpos($!reified,$!i),Nil))
                       while nqp::islt_i($!i = nqp::add_i($!i,1),$elems);
                     IterationEnd
                 }

@p6rt
Copy link
Author

p6rt commented Jun 4, 2016

From @lizmat

The weird thing here is, is that somehow the List.iterator is used, instead of the Array.iterator (which *does* take null values into account, even makes them bindable)

Is this still a problem on HEAD ?

Liz

On 04 Jun 2016, at 20​:45, Christian Bartolomaeus (via RT) <perl6-bugs-followup@​perl.org> wrote​:

# New Ticket Created by Christian Bartolomaeus
# Please include the string​: [perl #​128320]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl6/Ticket/Display.html?id=128320 >

The following code (tested in S32-array/delete.t) started to fail with rakudo-j recently​:

$ perl6-j -e 'my @​a = 0..1; @​a[0]​:delete; map { 1 }, @​a'
java.lang.NullPointerException
in block <unit> at -e line 1

$ perl6-j --ll-exception -e 'my @​a = 0..1; @​a[0]​:delete; map { 1 }, @​a'
java.lang.NullPointerException
in add_to_cache (gen/jvm/BOOTSTRAP.nqp​:1512)
in (gen/jvm/BOOTSTRAP.nqp​:1516)
in push (gen/jvm/CORE.setting​:1828)
in push-all (gen/jvm/CORE.setting​:14627)
in push-until-lazy (gen/jvm/CORE.setting​:2401)
in (gen/jvm/CORE.setting​:14308)
in (gen/jvm/CORE.setting​:14306)
in reify-until-lazy (gen/jvm/CORE.setting​:14304)
in is-lazy (gen/jvm/CORE.setting​:14892)
in map (gen/jvm/CORE.setting​:5366)
in map (gen/jvm/CORE.setting​:5365)
in <unit> (-e​:1)
in <unit-outer> (-e​:1)
in eval (gen/jvm/stage2/NQPHLL.nqp​:1198)
in eval (src/Perl6/Compiler.nqp​:171)
in (gen/jvm/stage2/NQPHLL.nqp​:1288)
in command_eval (gen/jvm/stage2/NQPHLL.nqp​:1285)
in command_eval (src/Perl6/Compiler.nqp​:29)
in command_line (gen/jvm/stage2/NQPHLL.nqp​:1269)
in MAIN (gen/jvm/main.nqp​:37)
in <mainline> (gen/jvm/main.nqp​:33)
in (gen/jvm/main.nqp)

I think, the error first appeared with Rakudo commit beb3c986.

The following patch makes the NPE go away (and has no spectest fallout). But I'm not sure if it is the right thing to insert Nil when nqp​::atpos returns null.

====

diff --git a/src/core/List.pm b/src/core/List.pm
index 0c65a6f..ddc8607 100644
--- a/src/core/List.pm
+++ b/src/core/List.pm
@​@​ -476,7 +476,7 @​@​ my class List does Iterable does Positional { # declared in BOOTSTRAP
method push-all($target) {
my int $elems = nqp​::elems($!reified);
my $no-sink;
- $no-sink := $target.push(nqp​::atpos($!reified,$!i))
+ $no-sink := $target.push(nqp​::ifnull(nqp​::atpos($!reified,$!i),Nil))
while nqp​::islt_i($!i = nqp​::add_i($!i,1),$elems);
IterationEnd
}

@p6rt
Copy link
Author

p6rt commented Jun 4, 2016

The RT System itself - Status changed from 'new' to 'open'

@p6rt
Copy link
Author

p6rt commented Jun 4, 2016

From @usev6

On Sat Jun 04 11​:45​:51 2016, bartolin@​gmx.de wrote​:

The following patch makes the NPE go away (and has no spectest
fallout). But I'm not sure if it is the right thing to insert Nil when
nqp​::atpos returns null.

However, PR created​: rakudo/rakudo#780

@p6rt
Copy link
Author

p6rt commented Jun 4, 2016

From @usev6

On Sat Jun 04 13​:20​:25 2016, elizabeth wrote​:

The weird thing here is, is that somehow the List.iterator is used,
instead of the Array.iterator (which *does* take null values into
account, even makes them bindable)

Is this still a problem on HEAD ?

Oops, didn't see your reply before posting my comment.

Yes, it's still a problem on HEAD.

I'm not really sure, but I think the NPE happens because somewhere in 'sub map' or 'method map' is a call to .list and we end up in List.iterator.

Without the 'map' there is no problem​:

$ perl6-j -e 'my @​array = 0..1; @​array[0]​:delete; say @​array'
[(Any) 1]

@p6rt
Copy link
Author

p6rt commented Jun 4, 2016

From @usev6

On Sat Jun 04 13​:30​:58 2016, bartolin@​gmx.de wrote​:

I'm not really sure, but I think the NPE happens because somewhere in
'sub map' or 'method map' is a call to .list and we end up in
List.iterator.

Ah, here (src/core/Any-iterable-methods.pm​:1371) is what made me assume that map caused us to end up in List.iterator -- values seems to be a List here​:

multi sub map(&code, +values) { my $laze = values.is-lazy; values.map(&code).lazy-if($laze) }

@p6rt
Copy link
Author

p6rt commented Jun 5, 2016

From @usev6

On Sat Jun 04 14​:10​:20 2016, bartolin@​gmx.de wrote​:

Ah, here (src/core/Any-iterable-methods.pm​:1371) is what made me
assume that map caused us to end up in List.iterator -- values seems
to be a List here​:

multi sub map(&code, +values) { my $laze = values.is-lazy;
values.map(&code).lazy-if($laze) }

Another thing to note​: Only the sub form of map has the problem​:

$ perl6-j -e 'my @​array = 0..1; @​array[0]​:delete; say map { $_ }, @​array'
java.lang.NullPointerException
  in block <unit> at -e line 1

$ perl6-j -e 'my @​array = 0..1; @​array[0]​:delete; say @​array.map( { $_ } )'
((Any) 1)

See also my latest comment on the PR​: rakudo/rakudo#780 (comment)

@p6rt
Copy link
Author

p6rt commented Sep 20, 2016

From @usev6

Yesterday we had the following discussion on #perl6-dev about this ticket​:

==== start of discussion on IRC -- cmp. http://irclog.perlgeek.de/perl6-dev/2016-09-19#i_13238477
bartolin lizmat​: I saw your commit 9b6f2eb543 in the backlog. does that shed a different light on RT #​128320 and the related rakudo PR 780? (that PR has conflicts now)
synopsebot6 Link​: https://rt-archive.perl.org/perl6//Public/Bug/Display.html?id=128320
lizmat bartolin​: looking
lizmat bartolin​: I don't think so, because #​128320 is about Arrays not Lists
lizmat ah, reading the pull request​: perhaps the jvm has problems pushing nqp​::null to an array ?
timotimo we don't have something like VMNull on JVM? that surprises me
arnsholt IIRC nqp​::null on JVM is just plain null
lizmat well, that's basically what https://github.com/rakudo/rakudo/pull/780/files does​: make sure we don't push nqp​::null
arnsholt There are two potential solutions here, IMO
arnsholt Either guard against the null from Rakudo (like in the PR), or find the spot in the Java runtime code that doesn't check for null and add a null check there instead
bartolin lizmat​: IIRC the problem occured because we used map in the failing code -- and inside map the List iterator is used
arnsholt I'd be inclined to go for finding the problem in the Java code, I think
bartolin sounds fair. I'll add this discussion to the ticket (and close the PR)
bartolin arnsholt++ lizmat++ # thanks for looking
lizmat bartolin​: if this is still a problem, then I could make it JVM only
bartolin lizmat​: no, it's not a big problem (not worth a special case IMHO)
lizmat ok
bartolin (I fudged the failing test as todo some time ago)
lizmat well, if it's todoed, is no longer NPEing ?
lizmat *it's
bartolin it's a 'lives-ok' test :-)
lizmat but still, an NPE would exit the test script, wouldn't be catchable ?
timotimo not sure if we turn an NPE into a perl6-level catchable exception
bartolin well, the test file (S32-array/delete.t) has no passed todos and it does not die, either
* lizmat is building an up-to-date JVM backend
lizmat bartolin​: seems the problem on JVM is now also gone​:
lizmat $ ./perl6-j -e 'my @​a; @​a[1] = 42; say @​a.map​: { 1 }'
lizmat (1 1)
lizmat oops, it isn't​:
lizmat $ ./perl6-j -e 'my @​a; @​a[1] = 42; say map { $_ }, @​a'
lizmat java.lang.NullPointerException
lizmat in block <unit> at -e line 1
bartolin yepp, it's only the sub form of map
lizmat m​: sub a(+v) { dd v }; a( my @​ = ^10 )
camelia rakudo-moar 77a7a4​: OUTPUT«(0, 1, 2, 3, 4, 5, 6, 7, 8, 9)␤»
lizmat m​: sub a(+@​v) { dd @​v }; a( my @​ = ^10 )
camelia rakudo-moar 77a7a4​: OUTPUT«[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]␤»
lizmat seems like adding a sigil makes it an Array, and thus circumvent the issue
bartolin lizmat​: you mean in multi sub map?
lizmat yeah...
lizmat testing that now
bartolin hmm, Larry introduced the +value there exactly one year ago (d9c21e99f5)
lizmat ./perl6-j -e 'my @​a; @​a[1] = 42; say map { $_ }, @​a'
lizmat ((Mu) 42)
lizmat bartolin​: alas, no go
lizmat m​: my @​a = ^10; map { $_ = 42 }, @​a; dd @​a # this breaks
camelia rakudo-moar 77a7a4​: OUTPUT«Array @​a = [42, 42, 42, 42, 42, 42, 42, 42, 42, 42]␤»
lizmat apparently the array is a copy
bartolin ah.
bartolin anyway, it's not the biggest problem with the jvm backend ... but thanks for looking!
lizmat bartolin​: yw
==== end of discussion on IRC -- powered by https://github.com/usev6/dump-irc-logs

1 similar comment
@p6rt
Copy link
Author

p6rt commented Sep 20, 2016

From @usev6

Yesterday we had the following discussion on #perl6-dev about this ticket​:

==== start of discussion on IRC -- cmp. http://irclog.perlgeek.de/perl6-dev/2016-09-19#i_13238477
bartolin lizmat​: I saw your commit 9b6f2eb543 in the backlog. does that shed a different light on RT #​128320 and the related rakudo PR 780? (that PR has conflicts now)
synopsebot6 Link​: https://rt-archive.perl.org/perl6//Public/Bug/Display.html?id=128320
lizmat bartolin​: looking
lizmat bartolin​: I don't think so, because #​128320 is about Arrays not Lists
lizmat ah, reading the pull request​: perhaps the jvm has problems pushing nqp​::null to an array ?
timotimo we don't have something like VMNull on JVM? that surprises me
arnsholt IIRC nqp​::null on JVM is just plain null
lizmat well, that's basically what https://github.com/rakudo/rakudo/pull/780/files does​: make sure we don't push nqp​::null
arnsholt There are two potential solutions here, IMO
arnsholt Either guard against the null from Rakudo (like in the PR), or find the spot in the Java runtime code that doesn't check for null and add a null check there instead
bartolin lizmat​: IIRC the problem occured because we used map in the failing code -- and inside map the List iterator is used
arnsholt I'd be inclined to go for finding the problem in the Java code, I think
bartolin sounds fair. I'll add this discussion to the ticket (and close the PR)
bartolin arnsholt++ lizmat++ # thanks for looking
lizmat bartolin​: if this is still a problem, then I could make it JVM only
bartolin lizmat​: no, it's not a big problem (not worth a special case IMHO)
lizmat ok
bartolin (I fudged the failing test as todo some time ago)
lizmat well, if it's todoed, is no longer NPEing ?
lizmat *it's
bartolin it's a 'lives-ok' test :-)
lizmat but still, an NPE would exit the test script, wouldn't be catchable ?
timotimo not sure if we turn an NPE into a perl6-level catchable exception
bartolin well, the test file (S32-array/delete.t) has no passed todos and it does not die, either
* lizmat is building an up-to-date JVM backend
lizmat bartolin​: seems the problem on JVM is now also gone​:
lizmat $ ./perl6-j -e 'my @​a; @​a[1] = 42; say @​a.map​: { 1 }'
lizmat (1 1)
lizmat oops, it isn't​:
lizmat $ ./perl6-j -e 'my @​a; @​a[1] = 42; say map { $_ }, @​a'
lizmat java.lang.NullPointerException
lizmat in block <unit> at -e line 1
bartolin yepp, it's only the sub form of map
lizmat m​: sub a(+v) { dd v }; a( my @​ = ^10 )
camelia rakudo-moar 77a7a4​: OUTPUT«(0, 1, 2, 3, 4, 5, 6, 7, 8, 9)␤»
lizmat m​: sub a(+@​v) { dd @​v }; a( my @​ = ^10 )
camelia rakudo-moar 77a7a4​: OUTPUT«[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]␤»
lizmat seems like adding a sigil makes it an Array, and thus circumvent the issue
bartolin lizmat​: you mean in multi sub map?
lizmat yeah...
lizmat testing that now
bartolin hmm, Larry introduced the +value there exactly one year ago (d9c21e99f5)
lizmat ./perl6-j -e 'my @​a; @​a[1] = 42; say map { $_ }, @​a'
lizmat ((Mu) 42)
lizmat bartolin​: alas, no go
lizmat m​: my @​a = ^10; map { $_ = 42 }, @​a; dd @​a # this breaks
camelia rakudo-moar 77a7a4​: OUTPUT«Array @​a = [42, 42, 42, 42, 42, 42, 42, 42, 42, 42]␤»
lizmat apparently the array is a copy
bartolin ah.
bartolin anyway, it's not the biggest problem with the jvm backend ... but thanks for looking!
lizmat bartolin​: yw
==== end of discussion on IRC -- powered by https://github.com/usev6/dump-irc-logs

@p6rt
Copy link
Author

p6rt commented Sep 10, 2017

From @usev6

This problem has been resolved​:

$ ./perl6-j -e 'my @​array = 0..1; @​array[0]​:delete; say map { $_ }, @​array'
((Any) 1)

The test in S32-array/delete.t is passing on the JVM backend as well. I'm closing this ticket as 'resolved'.

1 similar comment
@p6rt
Copy link
Author

p6rt commented Sep 10, 2017

From @usev6

This problem has been resolved​:

$ ./perl6-j -e 'my @​array = 0..1; @​array[0]​:delete; say map { $_ }, @​array'
((Any) 1)

The test in S32-array/delete.t is passing on the JVM backend as well. I'm closing this ticket as 'resolved'.

@p6rt
Copy link
Author

p6rt commented Sep 10, 2017

@usev6 - Status changed from 'open' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JVM Related to Rakudo-JVM
Projects
None yet
Development

No branches or pull requests

1 participant