-
Bug
-
Resolution: Not an Issue
-
P1
-
None
-
1.4.0
-
generic
-
generic
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".
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".