New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StackOverflowException #92

Closed
darkl opened this Issue Aug 24, 2015 · 3 comments

Comments

Projects
None yet
2 participants
@darkl
Copy link
Member

darkl commented Aug 24, 2015

(Related to #91, @nj4x)

Instructions how to reproduce:

Use this client and this router code.

Open a few (~3) tabs of the client in Chrome. Then open one client's Console. Close that client after the numbers in the Console get updated a few times. The Router will terminate with StackOverflowException.

Seems that the Exception is due to the ActionBlock implementation. Consider using Tpl Dataflow instead of this rx alternative.

@darkl darkl added bug router labels Aug 24, 2015

@darkl darkl added this to the WampSharp v1.2.3.* milestone Aug 24, 2015

@darkl

This comment has been minimized.

Copy link
Member Author

darkl commented Aug 24, 2015

Seems that Observable.Merge is implemented using recursion. If the queue is large enough, it causes a StackOverflowException.

@nj4x

This comment has been minimized.

Copy link

nj4x commented Aug 24, 2015

Hi Elad,

Please check https://github.com/nj4x/WampSharpTests.git project.
I've tried to collect multiple WampSharp issues in the single application.
You just need to download it and run to reproduce (note case 2 & 3 are
fixed in 1.2.2.53-dev)

Acc. to StackOverflow error - I think that changing rx to alternative will
result in other memory issues due to inability of Publisher to control over
the rate it publishes events.
Publisher should be able to check (in some way) pending events queue size
to detect and handle congestions. Is it possible to apply some
acknowledgement mechanisms without affecting performance and keeping
Publishers interface as simple as ((ISubject) _subject).OnNext(evt); ?

On Mon, 24 Aug 2015 at 23:23 Elad Zelingher notifications@github.com
wrote:

Seems that Observable.Merge is implemented using recursion. If the queue
is large enough, it causes a StackOverflowException.


Reply to this email directly or view it on GitHub
#92 (comment)
.

@darkl

This comment has been minimized.

Copy link
Member Author

darkl commented Aug 24, 2015

Hi @nj4x,

Thanks for the test project, I'll try to run it sometimes this week (probably in the weekend or so). I think you should still create an issue per problem so it would easier (for both of us) to track their statuses and discuss them separately.

About the queue stuff - queues will stay individual per client, so I don't find a lot of sense in exposing queue size to library users. I might consider making the ActionBlock mechanism plugable so users can have control on how items are queued per client, although I don't really like this.

Elad

@darkl darkl closed this Sep 15, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment