[jboss-jira] [JBoss JIRA] Commented: (JGRP-1271) MuxMessageDispatcher mutates RequestOptions leading ultimately to potential StackOverflowError

[Benoit Leblanc (JIRA)]

    [ https://issues.jboss.org/browse/JGRP-1271?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12629384#comment-12629384 ] 

Benoit Leblanc commented on JGRP-1271:
--------------------------------------

I wrote this [message|https://sourceforge.net/mailarchive/forum.php?thread_name=CAGvoXupS5guCA35TUFPA6zDg1fDWGvL%3D_%2B%2BxHgtW9_DZCEj4vQ%40mail.gmail.com&forum_name=javagroups-users] before seeing this issue.

The fix was applied to MuxMessageDispatcher but not to MuxRpcDispatcher. That's why StackOverflowError still occur.

Hope to have your feedback on my remarks.

> MuxMessageDispatcher mutates RequestOptions leading ultimately to potential StackOverflowError
> ----------------------------------------------------------------------------------------------
>
>                 Key: JGRP-1271
>                 URL: https://issues.jboss.org/browse/JGRP-1271
>             Project: JGroups
>          Issue Type: Bug
>    Affects Versions: 2.11
>         Environment: All
>            Reporter: Eric Sirianni
>            Assignee: Bela Ban
>             Fix For: 2.11.1, 2.12
>
>
> The presence of 'public static final' RequestOptions SYNC and ASYNC implies strongly that RequestOptions are immutable.  Otherwise, clients could be changing the meaning of these shared constants.
> However, MuxMessageDispatcher.cast() mutates the passed-in RequestOptions to set the RspFilter.  It chains the existing RspFilter in the RequestOptions to a new one:
>    options.setRspFilter((filter != null) ? new NoMuxHandlerRspFilter(filter) : new NoMuxHandlerRspFilter())
> The result of this is that the following innocent looking code code will quickly lead to a stack overflow as the RspFilter chain in the  RequestOptions.ASYNC object grows without bound:
> while (true) {
>    muxMessageDispatcher.cast(dests, msg, RequestOptions.ASYNC, false);
> }
> The workaround is to create a new RequestOptions object per call to cast() :
> while (true) {
>    muxMessageDispatcher.cast(dests, msg, new RequestOptions(...), false);
> }
> I recommend either:
> A. Deprecating the public static final RequestOptions ASYNC and SYNC fields and heavily JavaDoc'ing that clients must use a fresh RequestOptions for each send() or cast() invocation.
> -or-
> B. JavaDoc'ing the RequestOptions class as *immutable* and fixing MuxMessageDispatcher and other such JGroups blocks that mutate RequestOptions.  You may wish to add a "copy constructor" to RequestOptions to facilitate this.  The fix for MuxMessageDispatcher is fairly easy - just "clone" the passed-in RequestOptions and set the NoMuxHandlerRspFilter on the new RequestOptions object:
> <         return super.cast(dests, msg, options.setRspFilter((filter != null) ? new NoMuxHandlerRspFilter(filter) : new NoMuxHandlerRspFilter()), blockForResults);
> ---
> >         RequestOptions newOptions = new RequestOptions(options.getMode(), options.getTimeout(), options.getAnycasting(), options, (filter != null) ? new NoMuxHandlerRspFilter(filter) : new NoMuxHandlerRspFilter(), options.getFlags());
> >         return super.cast(dests, msg, newOptions, blockForResults);

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira