Uploaded image for project: 'MINA'
  1. MINA
  2. DIRMINA-994

The ConnectionRequest.cancel() method is inconsistent wrt concurrent access

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0.9
    • Fix Version/s: 2.0.10
    • Component/s: None
    • Labels:
      None

      Description

      When we call the ConnectionRequest.cancel() method, it does :

              public void cancel() {
                  if (!isDone()) {
                      super.cancel();
                      cancelQueue.add(this);
                      startupWorker();
                      wakeup();
                  }
      

      The problem is that the isDone() method is :

          public boolean isDone() {
              synchronized (lock) {
                  return ready;
              }
          }
      

      and the super.cancel() method is :

          public void cancel() {
              setValue(CANCELED);
          }
      

      with the setValue() method :

          public void setValue(Object newValue) {
              synchronized (lock) {
                  // Allowed only once.
                  if (ready) {
                      return;
                  }
      
                  result = newValue;
                  ready = true;
                  ...
      

      At this point, we may have the 'ready' flag set to false, enter into the super.cancel() method, and have the 'ready' flag set to true when we check it again, but still we will add the current connectRequest into the cancel queue, when we should not.

        Activity

        Hide
        elecharny Emmanuel Lecharny added a comment -

        Fixed with http://git-wip-us.apache.org/repos/asf/mina/commit/f1972fc3

        We now check the cancel() method return before adding the future to the list of cancelled ones.

        Show
        elecharny Emmanuel Lecharny added a comment - Fixed with http://git-wip-us.apache.org/repos/asf/mina/commit/f1972fc3 We now check the cancel() method return before adding the future to the list of cancelled ones.

          People

          • Assignee:
            Unassigned
            Reporter:
            elecharny Emmanuel Lecharny
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development