Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-4739743

Over generous use of the treeLock causing deadlock.

XMLWordPrintable

      This bug seriously effects the ability to handle synchronous event
      notifications for accessibility, it really needs to be fixed as soon
      as possible.

      I've noticed a deadlock situation that is seriously breaking Java when used
      with the JABG.

      The problem is probably best described by example, here are the sequence of
      events in an already running Java app:


      JAVA AT

      Dialog is brough up
        obtain tree lock - success
          children changed event is
          fired (wait for return)

                                              receives event
                                                since children changed get new
                                                children.
                                                  request for child @index 0
      request for child @index 0
        attempt to access children in tree
          obtain tree lock - failed
            wait for release - DEADLOCK!

      As you can see, within Java the deadlock is occurring because the method that
      dispatched the event did so while still holding the lock, and since the events
      are now synchronous in at-spi (which will not be changed back) then the
      delivery of the event and subsequent handling is causing the AT (in my case
      at-poke) to send back a request to the originating object (holding the lock)
      to ask it about the children that have changed... In doing so the method that
      tries to return the children deadlocks because the lock is still held in a
      different thread by method that's sending the event.

      The offending areas are mainly within java.awt.Container. The specfic
      methods are:

         protected void addImpl(Component comp, Object constraints, int index) {
          synchronized (getTreeLock()) { <<---- NOTE LOCK
             ......... SNIP 8< ...........
                  if (containerListener != null ||
                      (eventMask & AWTEvent.CONTAINER_EVENT_MASK) != 0 ||
                      Toolkit.enabledOnToolkit(AWTEvent.CONTAINER_EVENT_MASK)) {
                                           ContainerEvent.COMPONENT_ADDED,
                                           comp);
                      dispatchEvent(e);
                  }
             ......... SNIP 8< ...........
              }
          }

      and

          Accessible getAccessibleChild(int i) {
              synchronized (getTreeLock()) { <<---- NOTE LOCK
                  Component[] children = this.getComponents();
                  int count = 0;
                  for (int j = 0; j < children.length; j++) {
                      if (children[j] instanceof Accessible) {
                          if (count == i) {
                              return (Accessible) children[j];
                          } else {
                              count++;
                          }
                      }
                  }
                  return null;
              }
          }

      As you can see the treeLock is held for the WHOLE duration of the two
      for something as big as addImpl is in this case it's just silly.

      The main rule of thumb for locks like this are that they should ONLY be held
      when necessary and for as short a time as possible.

      What worries me is that this over generous use of the lock may give rise to more
      of these deadlock which we've just not found yet, the "tree lock" seems
      to be used all over in a way that uses a lock for duration of a complete
      method, when in reality most of the time we're not even accessing the "tree".




            lmonsantsunw Lynn Monsanto (Inactive)
            darrenk Darren Kenny
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated:
              Resolved:
              Imported:
              Indexed: