search

ReactiveX / RxCpp

Reactive Extensions for C++
Apache License 2.0
3.07k stars 396 forks source link

group_by with not completed observable issue #403

Open diorcety opened 7 years ago

diorcety commented 7 years ago

I found an potential issue with flat_map operation when source the is a scoped observable using a factory holding a shared pointer. In this case my shared pointer is not released after the last unsubscribe of the observable returned by flat_map. I didn't succeed to produce a minimal example that trigger this issue, but a quick and dirty hack seems to solve the issue: if I "reset" the state_type source (which is manly not used): this->source = source_type(); my smart pointer is released when I unsubscribe. Maybe this is an issue on my side, but I go many times through my code, I don't see the issue

kirkshoop commented 7 years ago

Hard to say without specific repro, but I would assume that something has stored the flat_map. the source must stay valid while the flat_map observable exists because it has to assume that subscribe might be called again.

diorcety commented 7 years ago

The source is still valid in the initial variable. I will try harder to create minimal example

diorcety commented 7 years ago

Here the issue: https://gist.github.com/diorcety/4cbded6fd3583ae001ebc9cf1d3e409d

diorcety commented 7 years ago

Commented on_completed:

Hello, World!
Abcd
Value: 0
Value: 20
Unsubscribe
End of World!

Uncommented

Hello, World!
Abcd
Value: 0
Value: 20
~Abcd
Unsubscribe
End of World!
diorcety commented 7 years ago

The issue is certainly in the group_by not the flat_map

diorcety commented 7 years ago

In both case at the unsubscribe, the hook (held by the lambda) should be released. Because there is no link to the observable anymore

kirkshoop commented 7 years ago

this is probably a bug in the lifetime - possibly introduced by (https://github.com/Reactive-Extensions/RxCpp/commit/8065a799545f6541f2ef95f38c2743bc92c54c8b#diff-9430ecceb5eeb688548de212b7fd57be)

Pure speculation, but it may be similar to the bug in window_toggle fixed here (https://github.com/Reactive-Extensions/RxCpp/commit/b41b15f1daf97519433742ff86adad9d0ae8acdd)

I will add this to the backlog - thank you for the report!

as a workaround until this bug is fixed, adding a take_until before the group_by where the trigger parameter was a subject whose on_next was called each time the cancellation was desired should complete the stream cleanly.

diorcety commented 7 years ago

I was on a old version, seems to be solved in the master