[Issue 631] New - BehaviorStructure : 'awtEventsBuffer' is not fully synchronized !

[interactivemesh]

https://java3d.dev.java.net/issues/show_bug.cgi?id=631
                 Issue #|631
                 Summary|BehaviorStructure : 'awtEventsBuffer' is not fully syn
                        |chronized !
               Component|java3d
                 Version|1.5.2
                Platform|All
              OS/Version|All
                     URL|
                  Status|NEW
       Status whiteboard|
                Keywords|
              Resolution|
              Issue type|DEFECT
                Priority|P3
            Subcomponent|j3d-core
             Assigned to|issues@java3d
             Reported by|interactivemesh






------- Additional comments from interactivemesh@... Mon Oct 26 14:22:59 +0000 2009 -------
The access to the UnorderList 'awtEventsBuffer' is not fully synchronized in the
class BehaviorStructure.

An AWTEvent can get lost when it is added to 'awtEventsBuffer' between the calls
'awtEventsBuffer.toArray()' and 'awtEventsBuffer.clear()'. This can happen to a
KEY_RELEASED event after a sequence of repeated KEY_PRESSED events.


// Called from Canvas3D.sendEventToBehaviorScheduler(AWTEvent evt)

void handleAWTEvent(AWTEvent evt) {

    awtEventsBuffer.add(evt);         // not synchronized !!!!

    VirtualUniverse.mc.sendRunMessage(universe, J3dThread.BEHAVIOR_SCHEDULER);
}

// Called from BehaviorScheduler.doWork

void handleAWTEvent() {

// ...

    synchronized (awtEventsBuffer) {
        events = (AWTEvent []) awtEventsBuffer.toArray();
        awtBufferSize = awtEventsBuffer.size();
        awtEventsBuffer.clear();
    }

// ...

}

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@...
For additional commands, e-mail: issues-help@...

[mcneillk]

https://java3d.dev.java.net/issues/show_bug.cgi?id=631






------- Additional comments from mcneillk@... Tue Oct 27 14:27:40 +0000 2009 -------
Some investigation reveals that the implementation of method

awtEventsBuffer.add()

is synchronized internally (in UnorderList.java). Also, methods

awtEventsBuffer.toArray()
awtEventsBuffer.clear()

are also synchronized in the same class.

So, in theory, there is no risk of losing events. In fact, it would be cleaner
to remove the external synchronized block

synchronized (awtEventsBuffer) {
        events = (AWTEvent []) awtEventsBuffer.toArray();
        awtBufferSize = awtEventsBuffer.size();
        awtEventsBuffer.clear();
    }

to avoid any confusion and unnecessary object locking.

I recommend either rejecting this bug, or re-qualifying it to an enhancement,
removing the extra synchronization, and changing "awtBufferSize =
awtEventsBuffer.size()" to "awtBufferSize = events.length()" for complete
correctness and readability.


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@...
For additional commands, e-mail: issues-help@...

[interactivemesh]

https://java3d.dev.java.net/issues/show_bug.cgi?id=631






------- Additional comments from interactivemesh@... Thu Oct 29 12:54:40 +0000 2009 -------
Sorry, I disregarded that the concerned methods are synchronized.

I recommend to leave the code unchanged for now, because

- awtEventsBuffer.size() is the exact number of events
- awtEventsBuffer.size() <= events.length, see UnorderList

- the synchronized block (nested locks) prevents the waiting event-dispatching
thread from adding an event while the BehaviorScheduler thread firstly creates
an array from the list and secondly clears the list in two separated calls.


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@...
For additional commands, e-mail: issues-help@...

[aces-2]

https://java3d.dev.java.net/issues/show_bug.cgi?id=631



User aces changed the following:

                What    |Old value                 |New value
================================================================================
                  Status|NEW                       |RESOLVED
--------------------------------------------------------------------------------
              Resolution|                          |INVALID
--------------------------------------------------------------------------------




------- Additional comments from aces@... Fri Oct 30 10:30:12 +0000 2009 -------
Thanks .
Yes, your analysis is correct.
I'll close it as invalid.

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@...
For additional commands, e-mail: issues-help@...