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@26-ea/javafx.util.Subscription.lambda$and$0(Subscription.java:103)
> at
> javafx.base@26-ea/javafx.util.Subscription.lambda$and$0(Subscription.java:103)
> at
> javafx.base@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
