linkedin / parseq

Asynchronous Java made easier
Apache License 2.0
1.17k stars 266 forks source link

Fix broken implementation of ParTaskImpl::tasksFromIterable #347

Closed zackthehuman closed 8 months ago

zackthehuman commented 8 months ago

The tasksFromIterable method in ParTaskImpl needs to read the contents of an iterator and then create an array of the read tasks. This is an unfortunate performance/memory tradeoff of using these two things: iterators don't have a length and arrays need one. The implementation reads the iterator's contents into a collection and then turns that collection into an array. The problem is, it then tries to cast that array of Object into a more narrow type, which is not allowed. This always results in a ClassCastException if the value is non null. The exists tests did not cover this case.

The code has been fixed to take a less performant path and delegate the array creation to tasksFromCollection. A test has been written for the Iterable code path and verified through coverage analysis.

This should fix #209 .

Before this change, if you use a class that implements Iterable but does not also implement Collection with Task.par then you'll be met with an exception:

java.lang.ClassCastException: [Ljava.lang.Object; cannot be cast to [Lcom.linkedin.parseq.Task;
    at com.linkedin.parseq.ParTaskImpl.tasksFromIterable(ParTaskImpl.java:77)
    at com.linkedin.parseq.ParTaskImpl.<init>(ParTaskImpl.java:61)
    at com.linkedin.parseq.Tasks.par(Tasks.java:361)
    at com.linkedin.parseq.TestParTask.testIterableSeqWithMultipleElements(TestParTask.java:105)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:85)
    at org.testng.internal.Invoker.invokeMethod(Invoker.java:639)
    at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:816)
    at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1124)
    at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:125)
    at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:108)
    at org.testng.TestRunner.privateRun(TestRunner.java:774)
    at org.testng.TestRunner.run(TestRunner.java:624)
    at org.testng.SuiteRunner.runTest(SuiteRunner.java:359)
    at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:354)
    at org.testng.SuiteRunner.privateRun(SuiteRunner.java:312)
    at org.testng.SuiteRunner.run(SuiteRunner.java:261)

This is because the Iterable code path does some illegal casting of Object[] to Task[], which is not allowed.