<div dir="ltr">Adding to this, I investigated what was exactly happening in my case. There was a Subscription with a few thousands recursive subscriptions, which were created by `and`.<div>However, only a few of those subscriptions were still "active".</div><div>When calling Subscription.unsubscribe() in case of a combined subscription, the listeners will be removed, but the inner Subscription objects are not removed, so the combined subscription keeps having multiple inner subscriptions.</div><div>This is a different, but related issue. The following pattern is thus wrong and dangerous:</div><div>```</div><div>Subscription s = Subscription.EMPTY; </div><div>for (int i = 0; i < 100; i++) { </div><div>    s.unsubscribe();</div><div>    s = s.and(fooProperty.subscribe(val -> ...));</div><div>    s = s.and(barProperty.subscribe(val -> ...));</div><div> }</div><div>```</div><div><br></div><div>The `s.unsubscribe` keeps strong references to all previously added subscriptions. It has to recursively loop over a growing list of Subscription.unsubscribe() calls.</div><div>Adding `s = Subscription.EMPTY` after `s.unsubscribe()` fixes that problem.</div><div><br></div><div>So I believe there are 2 different issues:</div><div>1. as reported in the original post, the recursive approach in unsubscribe can lead to StackOverflowErrors in case of many chained subscriptions</div><div>2. it is not clear from the documentation that Subscription.unsubscribe() is not removing the subscriptions (it just unsubscribes, but keeps the whole structure on the heap).</div><div><br></div><div>- Johan</div><div><br></div></div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Wed, Aug 13, 2025 at 10:12 PM John Hendrikx <<a href="mailto:john.hendrikx@gmail.com">john.hendrikx@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">You're right, this should preferably not happen.  The implementation is<br>
simple, but it does limit how many subscriptions you can chain.<br>
<br>
The `combine` variant does not have the same issue.<br>
<br>
I can rewrite it to avoid recursion, but it needs to be done very<br>
carefully as the subscriptions chained with `and` basically form a<br>
graph, and unsubscribing an older reference should not unsubscribe more<br>
than it would have done before calling `and`.<br>
<br>
--John<br>
<br>
On 13/08/2025 12:06, Johan Vos wrote:<br>
> Hi,<br>
><br>
> The current implementation of Subscription.unsubscribe() uses<br>
> recursion to unsubscribe the chain of subscriptions. This can lead to<br>
> a StackOverflowError in case there are many chained subscriptions.<br>
> Running the following code demonstrates this:<br>
><br>
> ```<br>
>     void testSubs() {<br>
>         SimpleStringProperty prop = new SimpleStringProperty("simpel");<br>
>         Subscription s = prop.subscribe(() -> {});<br>
>         for (int i = 0; i < 100000; i++) {<br>
>             Subscription t = prop.subscribe(() -> {});<br>
>             s = s.and(t);<br>
>         }<br>
>         s.unsubscribe();<br>
>     }<br>
> ```<br>
><br>
> This results in<br>
> ```<br>
> java.lang.StackOverflowError<br>
> at<br>
> javafx.base@26-ea/javafx.util.Subscription.lambda$and$0(Subscription.java:103)<br>
> at<br>
> javafx.base@26-ea/javafx.util.Subscription.lambda$and$0(Subscription.java:103)<br>
> at<br>
> javafx.base@26-ea/javafx.util.Subscription.lambda$and$0(Subscription.java:103)<br>
> ...<br>
> ```<br>
><br>
> While it's unusual (and in most cases a very bad idea) to chain that<br>
> many Subscriptions, I don't think this should give a StackOverflow<br>
> Error. I believe it is better to avoid recursion in the<br>
> implementation. If people agree, I'll file an issue for this.<br>
><br>
> - Johan<br>
</blockquote></div>