Uploaded image for project: 'JGroups'
  1. JGroups
  2. JGRP-1271

MuxMessageDispatcher mutates RequestOptions leading ultimately to potential StackOverflowError

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Done
    • Icon: Major Major
    • 2.11.1, 2.12
    • 2.11
    • None
    • Hide

      while (true) {
      muxMessageDispatcher.cast(dests, msg, RequestOptions.ASYNC, false);
      }

      StackOverflowError will eventually ensue:
      java.lang.StackOverflowError: null
      at org.jgroups.blocks.mux.NoMuxHandlerRspFilter.isAcceptable(NoMuxHandlerRspFilter.java:25) ~[jgroups.jar:2.10.0.GA]
      at org.jgroups.blocks.mux.NoMuxHandlerRspFilter.isAcceptable(NoMuxHandlerRspFilter.java:25) ~[jgroups.jar:2.10.0.GA]
      at org.jgroups.blocks.mux.NoMuxHandlerRspFilter.isAcceptable(NoMuxHandlerRspFilter.java:25) ~[jgroups.jar:2.10.0.GA]
      at org.jgroups.blocks.mux.NoMuxHandlerRspFilter.isAcceptable(NoMuxHandlerRspFilter.java:25) ~[jgroups.jar:2.10.0.GA]
      at org.jgroups.blocks.mux.NoMuxHandlerRspFilter.isAcceptable(NoMuxHandlerRspFilter.java:25) ~[jgroups.jar:2.10.0.GA]
      at org.jgroups.blocks.mux.NoMuxHandlerRspFilter.isAcceptable(NoMuxHandlerRspFilter.java:25) ~[jgroups.jar:2.10.0.GA]

      Show
      while (true) { muxMessageDispatcher.cast(dests, msg, RequestOptions.ASYNC, false); } StackOverflowError will eventually ensue: java.lang.StackOverflowError: null at org.jgroups.blocks.mux.NoMuxHandlerRspFilter.isAcceptable(NoMuxHandlerRspFilter.java:25) ~ [jgroups.jar:2.10.0.GA] at org.jgroups.blocks.mux.NoMuxHandlerRspFilter.isAcceptable(NoMuxHandlerRspFilter.java:25) ~ [jgroups.jar:2.10.0.GA] at org.jgroups.blocks.mux.NoMuxHandlerRspFilter.isAcceptable(NoMuxHandlerRspFilter.java:25) ~ [jgroups.jar:2.10.0.GA] at org.jgroups.blocks.mux.NoMuxHandlerRspFilter.isAcceptable(NoMuxHandlerRspFilter.java:25) ~ [jgroups.jar:2.10.0.GA] at org.jgroups.blocks.mux.NoMuxHandlerRspFilter.isAcceptable(NoMuxHandlerRspFilter.java:25) ~ [jgroups.jar:2.10.0.GA] at org.jgroups.blocks.mux.NoMuxHandlerRspFilter.isAcceptable(NoMuxHandlerRspFilter.java:25) ~ [jgroups.jar:2.10.0.GA]

      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);

              rhn-engineering-bban Bela Ban
              sirianni_jira Eric Sirianni (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Created:
                Updated:
                Resolved: