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

If exceptions occur, the parallel collection framework should return one of them and not a random number, non-deterministically wrapped in a different exception type #7474

Closed
scabug opened this issue May 12, 2013 · 3 comments
Assignees
Milestone

Comments

@scabug
Copy link

scabug commented May 12, 2013

"What's wrong with an API which non-deterministically returns either A or B(Set(A, ...)?"

... which seems pretty much what the exception handling behavior of the parallel collections does.

If exceptions of type A occur, either an exception of type A or an exception of type B, wrapping multiple exceptions of A's, is observed.
Imho, this behavior is incredibly broken and so unintuitive, that even people writing tests don't handle it correctly. See files/run/t5375.scala.

Concerning “non-deterministic”:
How many exceptions are observed before the operation is aborted depends on the machine, the available cores and hyper-threading, the operating system, the threadpool implementation and configuration, the size of the collection and the runtime itself.

In fact, files/run/t5375.scala is failing on both jdk7u and Avian because of this issue.

After thinking about this, I propose just throwing the "first" exception which occurs.
This seems to be what C# and Java both do, too.

Even consistently returning CompositeThrowable doesn't make much sense (because we have fail-fast behaviour and don't wait until all tasks have finished or have thrown an exception).
Therefore, there is no useful semantic in having a CompositeThrowable which returns "some" exceptions. Either we gather and return all the exceptions or we just return one, instead of wrapping a non-deterministic number of exceptions into a completely unrelated wrapper type.

Considering that changing the parallel collection semantics in such a profound way is out of question, I propose the second option.

As soon as we are targeting Java > 7 we can use addSurpressed to add further exceptions which also occurred.

Mailing list thread: https://groups.google.com/d/topic/scala-internals/jlHg5NKruLU/discussion

@scabug
Copy link
Author

scabug commented May 12, 2013

Imported From: https://issues.scala-lang.org/browse/SI-7474?orig=1
Reporter: @soc
Affected Versions: 2.9.3, 2.10.1, 2.11.0-M2

@scabug
Copy link
Author

scabug commented May 18, 2013

@soc said:
scala/scala#2563

@scabug
Copy link
Author

scabug commented Jun 9, 2013

@soc said:
Merged in scala/scala@b88801f

@scabug scabug closed this as completed Jun 9, 2013
@scabug scabug added this to the 2.11.0-M4 milestone Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants