Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 430296 - [1.8] broken code triggers stack overflow in CaptureBinding18.isCompatibleWith
Summary: [1.8] broken code triggers stack overflow in CaptureBinding18.isCompatibleWith
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 4.4   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 4.4 RC1   Edit
Assignee: Stephan Herrmann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-13 11:43 EDT by Stephan Herrmann CLA
Modified: 2014-05-19 04:35 EDT (History)
3 users (show)

See Also:

Attachments
test & fix (3.54 KB, patch)
2014-03-13 19:37 EDT, Stephan Herrmann CLA 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Stephan Herrmann CLA 2014-03-13 11:43:00 EDT 
This is what I had during editing:

//---
import java.lang.annotation.*;
import java.util.*;
import java.util.function.*;
import java.util.stream.*;
interface MyCollector<T, A, R> extends Collector<T, A, R> { }

public abstract class AnnotationCollector {
        abstract <T, K, U, M extends Map<K, U>> 
    MyCollector<T, ?, M> toMap(Function<? super T, ? extends K> km, BinaryOperator<U> mf);
        
        void test(Stream<Annotation> annotations) {
                annotations
                        .collect(toMap(Annotation::annotationType,
                                        (Annotation first, Annotation second) -> first));
        }
        
        Map<String, Person> test2(Stream<Person> persons) {
                return persons.collect(Collectors.toMap((Person p) -> p.getLastName(),
                                                                Function::identity,
                                                        (p1, p2) -> p1));
        }
}

class Person {
        String getLastName() { return ""; }
}
//---

The compiler answer with

java.lang.StackOverflowError at org.eclipse.jdt.internal.compiler.lookup.CaptureBinding18.isCompatibleWith(CaptureBinding18.java:123)
Comment 1 Stephan Herrmann CLA 2014-03-13 11:43:45 EDT 
I'll investigate for 4.4
Comment 2 Stephan Herrmann CLA 2014-03-13 19:32:04 EDT 
Mh, stackoverflow isn't exactly nice.

Fix is simple:

We encounter a recursive CaptureBinding18. Some queries in this class already avoid stackoverflow using the flag "inRecursiveFunction", doing the same also in CB18.isCompatibleWith fixes this bug.

On the long run we can avoid this situation even earlier: the recursive CB18 is constructed from a type bound K#1 <: K#1 (an inference variable should be a subtype of itself). This type bound is legal but unnecessary, I will reduce it directly to TRUE.
Comment 3 Stephan Herrmann CLA 2014-03-13 19:37:33 EDT 
Created attachment 240884 [details]
test & fix

Srikanth, Jay:

I don't have the time to run all tests (RunAllJava8Tests are green).

If you think this is good to have in GA and have the time to test it, from my p.o.v its safe and good to have. So feel free to release if you like. Otherwise I'll take it forward after GA.

Point in favour: when it hits you, no editing is possible, your only option is: exit workbench, edit file externally, and restart.
Comment 4 Srikanth Sankaran CLA 2014-03-13 19:56:40 EDT 
I'll take a look later today. Thanks Stephan.
Comment 5 Srikanth Sankaran CLA 2014-03-14 02:08:52 EDT 
I'll first take this up for black box testing. Subsequently see if I have
time for review and then decide.
Comment 6 Srikanth Sankaran CLA 2014-03-14 04:11:13 EDT 
(In reply to Srikanth Sankaran from comment #5)
> I'll first take this up for black box testing. Subsequently see if I have
> time for review and then decide.

All JDT core tests went OK, Jay is doing additional testing ...
Comment 7 Jay Arthanareeswaran CLA 2014-03-14 10:57:03 EDT 
Testing went well and released the changes into BETA_JAVA8:

http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?h=BETA_JAVA8&id=1b7626b6bc39479837ae0cdb8edf077e6aad6481
Comment 8 Stephan Herrmann CLA 2014-03-14 12:02:15 EDT 
(In reply to Jayaprakash Arthanareeswaran from comment #7)
> Testing went well and released the changes into BETA_JAVA8:
> 
> http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/
> ?h=BETA_JAVA8&id=1b7626b6bc39479837ae0cdb8edf077e6aad6481

Thanks!

Polish/optimization mentioned in comment 2 will be handled via bug 430404
Comment 9 Jay Arthanareeswaran CLA 2014-05-19 01:32:38 EDT 
Somehow we ended up with target 4.4 even though this was released in BETA_JAVA8. To take it through verification stage, setting it to RC1.
Comment 10 shankha banerjee CLA 2014-05-19 04:26:24 EDT 
Verified for Luna 4.4 RC1 Build id: I20140518-2000.