You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "pjfanning (via GitHub)" <gi...@apache.org> on 2025/09/14 12:15:34 UTC
[PR] Convert non finite durations [pekko]
pjfanning opened a new pull request, #2219:
URL: https://github.com/apache/pekko/pull/2219
Basically we have a bug in pekko.util.JavaDurationConverters.
It allows you to compile:
```
import pekko.util.JavaDurationConverters._
scala.concurrent.duration.Duration.Inf.asJava
```
But this will fail at runtime because pekko.util.JavaDurationConverters calls toNanos on the scala duration and this fails with
```
java.lang.IllegalArgumentException: toNanos not allowed on infinite Durations
at scala.concurrent.duration.Duration$Infinite.fail(Duration.scala:219)
```
I found a case in pekko-http where we have a scala.concurrent.duration.Duration that we call asJava on and that could fail if the duration was set to a non-finite one (which is allowed).
Scala's builit-in converters for Durations only allow FiniteDuration instances to be converted while Pekko allows all Durations.
Java does not support Inifinite durations but this patch converts to the max allowed duration which is very long and safe enough.
Targetting 1.x as in 2.0.0, we will likely be deleting pekko.util.JavaDurationConverters
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] Convert non finite durations [pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #2219:
URL: https://github.com/apache/pekko/pull/2219#issuecomment-3289524700
> Myabe this can upstream to Scala/scala
Scala didn't make the same mistake we made. They only have implicit support for asJava on FiniteDuration. Pekko added it for all Durations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] Convert non finite durations [pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #2219:
URL: https://github.com/apache/pekko/pull/2219#issuecomment-3311835041
> Thanks @mdedetrich. I'm away from my computer today. I'll think about reverting this and will attempt to justify it otherwise. For 1.x, the code in this repo benefits from it. I found 3 affected cases in pekko-http and have created a pekko-http specific class to handle the issue there. The changes are in main branch of pekko-http only. I will check over the next few days if any affected code is at risk of trying to convert non-finite durations.
I would say that if we keep this PR, we would need to do a new release for every single Pekko satellite project because thats the only way we can actually state that this fix would work as it would only inline at compile time. Not saying that we should do all releases immediately, but we need to plan about doing at least one such release per satellite project. Its also a bit annoying as Pekko Connectors itself just had a 1.2.0 release, but it is what it is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] Convert non finite durations [pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #2219:
URL: https://github.com/apache/pekko/pull/2219#discussion_r2348176371
##########
actor/src/main/scala-3/org/apache/pekko/util/JavaDurationConverters.scala:
##########
@@ -34,6 +35,11 @@ private[pekko] object JavaDurationConverters {
}
final implicit class ScalaDurationOps(val self: Duration) extends AnyVal {
- def asJava: JDuration = JDuration.ofNanos(self.toNanos)
+ def asJava: JDuration = self match {
+ case fd: FiniteDuration => JDuration.ofNanos(fd.toNanos)
Review Comment:
The existing solution is `def asJava: JDuration = JDuration.ofNanos(self.toNanos)` and this has not caused problems. I know that scala-compat-java8 uses something more like your code but this PR is not about issues with finite durations, it is about issues with non-finite durations (infinite and undefined).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] Convert non finite durations [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #2219:
URL: https://github.com/apache/pekko/pull/2219#issuecomment-3311844100
@pjfanning So the inline is been enabled in Pekko 1.2?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] Convert non finite durations [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #2219:
URL: https://github.com/apache/pekko/pull/2219#discussion_r2347762673
##########
actor/src/main/scala-3/org/apache/pekko/util/JavaDurationConverters.scala:
##########
@@ -34,6 +35,11 @@ private[pekko] object JavaDurationConverters {
}
final implicit class ScalaDurationOps(val self: Duration) extends AnyVal {
- def asJava: JDuration = JDuration.ofNanos(self.toNanos)
+ def asJava: JDuration = self match {
+ case fd: FiniteDuration => JDuration.ofNanos(fd.toNanos)
Review Comment:
maybe something like :
```scala
if (duration.length == 0) JDuration.ZERO
else duration.unit match {
case TimeUnit.NANOSECONDS => JDuration.ofNanos(duration.length)
case TimeUnit.MICROSECONDS => JDuration.of(duration.length, ChronoUnit.MICROS)
case TimeUnit.MILLISECONDS => JDuration.ofMillis(duration.length)
case TimeUnit.SECONDS => JDuration.ofSeconds(duration.length)
case TimeUnit.MINUTES => JDuration.ofMinutes(duration.length)
case TimeUnit.HOURS => JDuration.ofHours(duration.length)
case TimeUnit.DAYS => JDuration.ofDays(duration.length)
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] Convert non finite durations [pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #2219:
URL: https://github.com/apache/pekko/pull/2219#issuecomment-3311861774
> @pjfanning So the inline is been enabled in Pekko 1.2?
Inlining has been enabled since 1.1.x (iirc)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] Convert non finite durations [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #2219:
URL: https://github.com/apache/pekko/pull/2219#issuecomment-3289517665
Myabe this can upstream to Scala/scala
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] Convert non finite durations [pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning merged PR #2219:
URL: https://github.com/apache/pekko/pull/2219
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] Convert non finite durations [pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #2219:
URL: https://github.com/apache/pekko/pull/2219#issuecomment-3311457916
> @He-Pin @mdedetrich @raboof I'm not sure whether this change helps much. The code with the bug (not being able to handle infinite or undefined Scala durations) is already inlined into our module jars. We haven't hit any issues yet but maybe it is safer to put this fix in, as at least it covers the code in this repo. I played around with long finite durations and found that the existing code for finite durations is adequate (just using nanos). I added comments in the code about the fact that converting from a Java Duration to a Scala Duration can fail with an IllegalArgumentException. Scala FiniteDurations max out at plus or minus 252 years while Java Durations support billion year durations. This is a pretty unlikely scenario. We do see Duration.Inf used in Scala though as it is relatively commonly used by people who don't want to apply a limit.
@pjfanning
Sorry I just noticed this now. Actually I am not that supportive of merging this in the 1.x series of Pekko because it can create really confusing behaviour with inlining (as you have pointed out) and I don't its been causing any problems in actual production (I understand that its easy to create a scenario where this can break but thats different to people experiencing it in prod).
Merging this PR for 2.0.x is absolutely fine, but with how inlining works for the 1.x series of Pekko, we can't even announce it as a normal bug fix because replacing the pekko jar wouldn't work, they would have also to replace the pekko satellite project in question.
What are your thoughts about reverting this change? I can still see your argument so I am not going to push strongly, but it is a really confusing experience for end users.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] Convert non finite durations [pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning closed pull request #2219: Convert non finite durations
URL: https://github.com/apache/pekko/pull/2219
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] Convert non finite durations [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #2219:
URL: https://github.com/apache/pekko/pull/2219#issuecomment-3311872697
Opps。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] Convert non finite durations [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #2219:
URL: https://github.com/apache/pekko/pull/2219#issuecomment-3301203808
Technically speaking, this change is correct now, but it may not be a problem in real scenarios. At least I haven't seen anyone report any problems in pekko or akka.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] Convert non finite durations [pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #2219:
URL: https://github.com/apache/pekko/pull/2219#issuecomment-3311721609
Thanks @mdedetrich. I'm away from my computer today. I'll think about reverting this and will attempt to justify it otherwise.
For 1.x, the code in this repo benefits from it. I found 3 affected cases in pekko-http and have created a pekko-http specific class to handle the issue there. The changes are in main branch of pekko-http only. I will check over the next few days if any affected code is at risk of trying to convert non-finite durations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] Convert non finite durations [pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #2219:
URL: https://github.com/apache/pekko/pull/2219#issuecomment-3291933016
@He-Pin @mdedetrich @raboof I'm not sure whether this change helps much. The code with the bug (not being able to handle infinite or undefined Scala durations) is already inlined into our module jars.
We haven't hit any issues yet but maybe it is safer to put this fix in, as at least it covers the code in this repo.
I played around with long finite durations and found that the existing code for finite durations is adequate (just using nanos).
I added comments in the code about the fact that converting from a Java Duration to a Scala Duration can fail with an IllegalArgumentException. Scala FiniteDurations max out at plus or minus 252 years while Java Durations support billion year durations. This is a pretty unlikely scenario.
We do see Duration.Inf used in Scala though as it is relatively commonly used by people who don't want to apply a limit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] Convert non finite durations [pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #2219:
URL: https://github.com/apache/pekko/pull/2219#issuecomment-3313714662
Do far, I've only seen the issue in pekko-http so one solution is that in pekko-http 1.3.0 that we fix the problematic conversions. They are fixed in main branch already.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org