StackOverflowError in recursive Subscription.unsubscribe

[John Hendrikx]

You're right, this should preferably not happen.  The implementation is
simple, but it does limit how many subscriptions you can chain.

The `combine` variant does not have the same issue.

I can rewrite it to avoid recursion, but it needs to be done very
carefully as the subscriptions chained with `and` basically form a
graph, and unsubscribing an older reference should not unsubscribe more
than it would have done before calling `and`.

--John

On 13/08/2025 12:06, Johan Vos wrote:
> Hi,
>
> The current implementation of Subscription.unsubscribe() uses
> recursion to unsubscribe the chain of subscriptions. This can lead to
> a StackOverflowError in case there are many chained subscriptions.
> Running the following code demonstrates this:
>
> ```
>     void testSubs() {
>         SimpleStringProperty prop = new SimpleStringProperty("simpel");
>         Subscription s = prop.subscribe(() -> {});
>         for (int i = 0; i < 100000; i++) {
>             Subscription t = prop.subscribe(() -> {});
>             s = s.and(t);
>         }
>         s.unsubscribe();
>     }
> ```
>
> This results in
> ```
> java.lang.StackOverflowError
> at
> javafx.base at 26-ea/javafx.util.Subscription.lambda$and$0(Subscription.java:103)
> at
> javafx.base at 26-ea/javafx.util.Subscription.lambda$and$0(Subscription.java:103)
> at
> javafx.base at 26-ea/javafx.util.Subscription.lambda$and$0(Subscription.java:103)
> ...
> ```
>
> While it's unusual (and in most cases a very bad idea) to chain that
> many Subscriptions, I don't think this should give a StackOverflow
> Error. I believe it is better to avoid recursion in the
> implementation. If people agree, I'll file an issue for this.
>
> - Johan