Improve `scala.util.Using` suppression order (NonFatal suppresses ControlThrowable) #11000

NthPortal · 2025-02-18T05:27:04Z

In scala.util.Using, when two exceptions are thrown, one is attached to the other using addSuppressed
according to a specified order.

Change that order to have scala.util.control.ControlThrowable suppressed by non-fatal exceptions (scala.util.control.NonFatal).

Fixes scala/bug#13088

Users of scala-collection-compat can upgrade to 2.14.0 to get the same behavior change

If you use Using in a project that crossbuilds to Scala 3, you may wish to wait for a Scala 3 release that includes the 2.13.17 standard library, or use a dependency override to force the upgrade in your build

som-snytt · 2025-02-18T06:05:26Z

Probably you didn't anticipate such far-reaching effects!

java.io.IOException: No space left on device
	at java.base/java.io.FileOutputStream.writeBytes(Native Method)
***
	at java.base/java.lang.Thread.run(Thread.java:1583)
ERROR: Maximum checkout retry attempts reached, aborting

lrytz · 2025-02-18T07:51:32Z

I'll fix Jenkins. GHA says

Test run scala.util.UsingTest finished: 4 failed

lrytz · 2025-02-18T08:49:32Z

Here's an example that changes semantics. Is that a good change?

Before:

scala> def h(s: String): Unit = throw new RuntimeException
scala> def t: Int = util.Using.resource("hai")(s => return 42)(h)
scala> t
val res0: Int = 42

After:

...
scala> t
java.lang.RuntimeException
  at h(<console>:1)
  at $anonfun$t$2(<console>:1)
  at scala.util.Using$.resource(Using.scala:304)
  at t(<console>:1)
  ... 30 elided
  Suppressed: scala.runtime.NonLocalReturnControl$mcI$sp

@som-snytt I don't follow your mUsings around Odersky's Rule and the Titanic, I'm afraid..

NthPortal · 2025-02-18T09:58:14Z

Is that a good change?

I would argue that not only is it a good change, it is a correct change. before the change, where did the RuntimeException go? did we not close the resource? was it eaten by a grue? we've somehow returned normally without handling the exception. after this change, the exception when closing the resource propagates as it should (and would if Using was language syntax like try-with-resources is and not just a regular method).

I'll have a look at the failing tests Soon™. I admittedly did not run them locally, and just kinda assumed I adjusted them correctly

som-snytt · 2025-02-18T10:19:18Z

In the example, we want the happy path.

If I want to "handle all the possibilities", then I have to catch all the possible aborts.

The answer for Future is use UEH so exceptions don't "disappear".

lrytz · 2025-02-18T12:39:11Z

Java:

jshell> public int f() throws Exception { try (Closeable c = () -> { throw new RuntimeException(); }) { return 42; } }
|  modified method f()

jshell> f()
|  Exception java.lang.RuntimeException
|        at lambda$f$0 (#3:1)
|        at f (#3:1)
|        at (#4:1)

som-snytt · 2025-02-18T17:41:09Z

The Odersky Rule is promulgated at
scala/scala3#17737 (comment)

som-snytt · 2025-02-18T17:49:32Z

scala> :pa -java
// Entering paste mode (ctrl-D to finish)

package j;
import java.io.Closeable;
public class C {
public int f() throws Exception { try (Closeable c = () -> { throw new RuntimeException(); }) { return 42; } }
}

// Exiting paste mode... now compiling with javac.

scala> new j.C().f()
java.lang.RuntimeException
  at j.C.lambda$f$0(C.java:4)
  at j.C.f(C.java:4)
  ... 30 elided

scala>

Ergonomics in repl could be improved.

Ichoran · 2025-02-18T18:26:32Z

The new behavior is correct, I think: if you throw an exception while closing the resource, you did not successfully use the resource.

However, non-local return is deprecated, and boundary throws a RuntimeException which is eaten by Try so to be consistently forgiving of people who let control flow escape their threads, Using needs to (and does) catch that one too, which makes any change to ControlThrowable-handling a matter of backwards compatibility only: there's no real purpose to ControlThrowable any longer, and code would run faster if we deprecate and eliminate it.

Therefore, I am not convinced that there's much benefit to changing Using in this way. The existing way is arguably wrong, but there shouldn't be much future for it, and someone may have relied upon returns working even if closing mucked up in ordinary ways. On the other hand, the danger of changing behavior in unpleasantly also seems modest.

lrytz · 2025-02-19T08:37:02Z

... so we cannot fix Using for boundary / break?

NthPortal · 2025-04-02T02:59:45Z

so we cannot fix Using for boundary / break?

personally, I think this PR is a bug fix. the previous/current behaviour is wrong I believe because I (and everyone else) didn't think about it particularly hard past "fatal should have priority", not because we explicitly decided that ControlThrowable specifically ought to have priority over non-fatal exceptions.

also, sorry for taking so long to fix the tests. life happened at me

Ichoran · 2025-04-02T04:05:29Z

... so we cannot fix Using for boundary / break?

We could (by intercepting the type of exception that they throw and handling them specially), but since boundary/break control flow can't get through Try by design (we had long arguments about that, and the side I favored lost)--then Using should eat it also for consistency. That way you just remember that standard library exception-handling stuff eats control flow.

And then if you want exception handling that allows control flow to cross, you use a library for that.

NthPortal · 2025-04-02T16:59:31Z

I don't think this PR changes anything meaningful wrt boundary/break given that it throws a RuntimeException

lrytz · 2025-04-03T07:43:17Z

since boundary/break control flow can't get through Try by design (we had long arguments about that, and the side I favored lost)--then Using should eat it also for consistency. That way you just remember that standard library exception-handling stuff eats control flow.

Thanks, that makes sense :-/

Change `scala.util.Using` to have `scala.util.control.ControlThrowable` be suppressed by non-fatal exceptions (`scala.util.control.NonFatal`).

NthPortal · 2025-04-03T08:27:03Z

thanks for squashing for me Lukas!

lrytz · 2025-04-03T10:12:17Z

Thank you for the work!

Improve `s.u.Using` suppression order

SethTisue · 2025-10-07T00:35:29Z

ported to scala-collection-compat in scala/scala-collection-compat#712

### What changes were proposed in this pull request? Upgrade to scala 2.13.17 ### Why are the changes needed? To bring the latest bug fixes and improvements like JDK 25 support. Note that Scala community announces two breaking changes due to the bug fixes. > Breaking changes > - Mix in the productPrefix hash statically in case class hashCode > - Improve scala.util.Using suppression order - https://github.com/scala/scala/releases/tag/v2.13.17 - scala/scala#11046 - scala/scala#10937 - scala/scala#10927 - scala/bug#13058 - scala/scala#11023 - scala/bug#13033 - scala/scala#11000 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? local and github builds ### Was this patch authored or co-authored using generative AI tooling? No Closes #52509 from vrozov/SPARK-53585. Authored-by: Vlad Rozov <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>

### What changes were proposed in this pull request? Upgrade to scala 2.13.17 ### Why are the changes needed? To bring the latest bug fixes and improvements like JDK 25 support. Note that Scala community announces two breaking changes due to the bug fixes. > Breaking changes > - Mix in the productPrefix hash statically in case class hashCode > - Improve scala.util.Using suppression order - https://github.com/scala/scala/releases/tag/v2.13.17 - scala/scala#11046 - scala/scala#10937 - scala/scala#10927 - scala/bug#13058 - scala/scala#11023 - scala/bug#13033 - scala/scala#11000 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? local and github builds ### Was this patch authored or co-authored using generative AI tooling? No Closes apache#52509 from vrozov/SPARK-53585. Authored-by: Vlad Rozov <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>

NthPortal added the library:base Changes to the base library, i.e. Function Tuples Try label Feb 18, 2025

scala-jenkins added this to the 2.13.17 milestone Feb 18, 2025

NthPortal force-pushed the using-pref/PR branch from d9a208f to 4169aa1 Compare February 18, 2025 05:29

NthPortal force-pushed the using-pref/PR branch from 4169aa1 to 312dcf4 Compare February 18, 2025 23:00

Improve s.u.Using suppression order

8091336

Change `scala.util.Using` to have `scala.util.control.ControlThrowable` be suppressed by non-fatal exceptions (`scala.util.control.NonFatal`).

lrytz force-pushed the using-pref/PR branch from bb60251 to 8091336 Compare April 3, 2025 07:44

lrytz approved these changes Apr 3, 2025

View reviewed changes

lrytz merged commit 483b4ee into scala:2.13.x Apr 3, 2025
3 checks passed

SethTisue added the release-notes worth highlighting in next release notes label Apr 3, 2025

NthPortal deleted the using-pref/PR branch April 4, 2025 00:47

SethTisue mentioned this pull request Apr 14, 2025

Update Using for Scala 2.13.17 scala/scala-collection-compat#692

Closed

hamzaremmal pushed a commit to hamzaremmal/scala3 that referenced this pull request May 2, 2025

Merge pull request scala/scala#11000 from NthPortal/using-pref/PR

262e595

Improve `s.u.Using` suppression order

hamzaremmal pushed a commit to scala/scala3 that referenced this pull request May 7, 2025

Merge pull request scala/scala#11000 from NthPortal/using-pref/PR

62d069e

Improve `s.u.Using` suppression order

lrytz changed the title ~~Improve s.u.Using suppression order~~ Improve scala.util.Using suppression order (NonFatal suppresses ControlThrowable) Sep 19, 2025

SethTisue mentioned this pull request Oct 7, 2025

update Using from Scala 2.13.17 and add UsingTest scala/scala-collection-compat#712

Merged

Divs-B mentioned this pull request Oct 9, 2025

chore(deps): Non-AWS dependency updates guardian/redirect-resolver#45

Merged

dongjoon-hyun mentioned this pull request Oct 13, 2025

[SPARK-53585][BUILD] Upgrade Scala to 2.13.17 apache/spark#52509

Closed

zhangyue19921010 mentioned this pull request Nov 1, 2025

[SPARK-53585][BUILD] Upgrade Scala to 2.13.17 OneTech-AI/fast-spark#20

Merged

Improve scala.util.Using suppression order (NonFatal suppresses ControlThrowable) #11000

Improve scala.util.Using suppression order (NonFatal suppresses ControlThrowable) #11000

Uh oh!

Conversation

NthPortal commented Feb 18, 2025 • edited by SethTisue Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

som-snytt commented Feb 18, 2025

Uh oh!

lrytz commented Feb 18, 2025

Uh oh!

lrytz commented Feb 18, 2025

Uh oh!

NthPortal commented Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

som-snytt commented Feb 18, 2025

Uh oh!

lrytz commented Feb 18, 2025

Uh oh!

som-snytt commented Feb 18, 2025

Uh oh!

som-snytt commented Feb 18, 2025

Uh oh!

Ichoran commented Feb 18, 2025

Uh oh!

lrytz commented Feb 19, 2025

Uh oh!

NthPortal commented Apr 2, 2025

Uh oh!

Ichoran commented Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NthPortal commented Apr 2, 2025

Uh oh!

lrytz commented Apr 3, 2025

Uh oh!

NthPortal commented Apr 3, 2025

Uh oh!

Uh oh!

lrytz commented Apr 3, 2025

Uh oh!

SethTisue commented Oct 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Improve `scala.util.Using` suppression order (NonFatal suppresses ControlThrowable) #11000

Improve `scala.util.Using` suppression order (NonFatal suppresses ControlThrowable) #11000

NthPortal commented Feb 18, 2025 •

edited by SethTisue

Loading

NthPortal commented Feb 18, 2025 •

edited

Loading

Ichoran commented Apr 2, 2025 •

edited

Loading