SwingWorker a special producer/consumer

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

SwingWorker a special producer/consumer

Remi Forax
Yesterday, i found a bug in the Sun implementation of the SwingWorker :
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6432565

Basically a swing worker permits to create a worker thread for
a long duration job that need to perform some refresh of the
swing ui without freezing the application.

Because only the Event Dispatch Thread can change the
swing UI, the worker thread publish data and
the EDT consume them.
see http://download.java.net/jdk6/docs/api/javax/swing/SwingWorker.html

It's a special producer/consumer because the EDT consume
all the available data in once.

Which data structure should be used in that case ?

Rémi Forax




_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: SwingWorker a special producer/consumer

Gregg Wonderly-2


Rémi Forax wrote:
> Yesterday, i found a bug in the Sun implementation of the SwingWorker :
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6432565

That bug is not visible yet, can you give specifics?

Gregg Wonderly

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: SwingWorker a special producer/consumer

Pete.Soper
Hi Rémi,
        Nice to see your mail again. Are you looking for a suggestion for a
thread-safe queue or the like to pass stuff between your EDT and your
main thread?

Regards,
Pete

Gregg Wonderly wrote:
>
> Rémi Forax wrote:
>
>>Yesterday, i found a bug in the Sun implementation of the SwingWorker :
>>http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6432565
>
>
> That bug is not visible yet, can you give specifics?

Still in dispatch:

FULL PRODUCT VERSION :
java version "1.6.0-beta2"
Java(TM) SE Runtime Environment (build 1.6.0-beta2-b85)
Java HotSpot(TM) Client VM (build 1.6.0-beta2-b85, mixed mode, sharing)


ADDITIONAL OS VERSION INFORMATION :
Linux localhost.localdomain 2.6.15-1.1833_FC4 #1 Wed Mar 1 23:41:37 EST
2006 i686 i686 i386 GNU/Linux


A DESCRIPTION OF THE PROBLEM :
The EDT (Event dispatch Thread) raises a ClassCastException
with the code below.

Data published by doInBackground() are stored in a list
and then converted in an array and send to method process()
in order to reduce synchronization time between EDT and worker thread.

The problem is to determine the class of the array passed to the process()
method at runtime. Because generics are erased, the class of V is not
available. The current implementation use the class of the array passed
as argument of the first call of the method publish() . This is clearly
wrong.

Two solutions :
   - don't use an array in the signature of process() but use a List instead
   - take the class of V in the constructor.
     public SwingWorker(Class<T> tClass) {...}

Note : i had reviewed the SwingWorker code when it was integrated to
mustang (i think a year ago), it troubles me because
it mix runtime type and generics but i was not able to produce a test case
that show my insight until today.
So the test case is not issue from a real application.

Rémi Forax

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
run the code

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
it should work
ACTUAL -
it throws a ClassCastException

ERROR MESSAGES/STACK TRACES THAT OCCUR :
Exception in thread "AWT-EventQueue-0" java.lang.ArrayStoreException
        at java.lang.System.arraycopy(Native Method)
        at java.util.ArrayList.toArray(ArrayList.java:306)
        at sun.swing.AccumulativeRunnable.flush(AccumulativeRunnable.java:148)
        at sun.swing.AccumulativeRunnable.run(AccumulativeRunnable.java:96)
        at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:209)
        at java.awt.EventQueue.dispatchEvent(EventQueue.java:598)
        at
java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:273)
        at
java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:183)
        at
java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:173)
        at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:168)
        at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:160)
        at java.awt.EventDispatchThread.run(EventDispatchThread.java:121)


REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
import javax.swing.SwingWorker;

public class SwingWorkerTest {
     public static void main(String[] args) {
       new SwingWorker<Void,CharSequence>() {
         @Override
         protected Void doInBackground() {
           publish(new String[] {"hello"});
           publish(new StringBuilder("world"));
           return null;
         }
       }.execute();
     }
}

---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
Add this line
publish(new CharSequence[0]);
as the first line of doInBackground().
*** (#1 of 1): 2006-05-31 19:56:56 EDT *.*@sun.com


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: SwingWorker a special producer/consumer

Remi Forax
Pete Soper a écrit :

> Hi Rémi,
>     Nice to see your mail again. Are you looking for a suggestion for
> a thread-safe queue or the like to pass stuff between your EDT and
> your main thread?
>
> Regards,
> Pete

The bug reported is not a concurrency bug but
because the implementation must be changed
perhaps the signature of the process() method
can be changed to take a thread safe List or a Queue.

The SwingWorker is clearly a producer/consumer problem :
- i want to put arrays of values (V)
- i want to take all the values in one call.
all in a thread safe way.

The question is what is the best concurrent data structure
for this job.

Rémi Forax

>
> Gregg Wonderly wrote:
>
>>
>> Rémi Forax wrote:
>>
>>> Yesterday, i found a bug in the Sun implementation of the SwingWorker :
>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6432565
>>
>>
>>
>> That bug is not visible yet, can you give specifics?
>
>
> Still in dispatch:
>
> FULL PRODUCT VERSION :
> java version "1.6.0-beta2"
> Java(TM) SE Runtime Environment (build 1.6.0-beta2-b85)
> Java HotSpot(TM) Client VM (build 1.6.0-beta2-b85, mixed mode, sharing)
>
>
> ADDITIONAL OS VERSION INFORMATION :
> Linux localhost.localdomain 2.6.15-1.1833_FC4 #1 Wed Mar 1 23:41:37
> EST 2006 i686 i686 i386 GNU/Linux
>
>
> A DESCRIPTION OF THE PROBLEM :
> The EDT (Event dispatch Thread) raises a ClassCastException
> with the code below.
>
> Data published by doInBackground() are stored in a list
> and then converted in an array and send to method process()
> in order to reduce synchronization time between EDT and worker thread.
>
> The problem is to determine the class of the array passed to the
> process()
> method at runtime. Because generics are erased, the class of V is not
> available. The current implementation use the class of the array passed
> as argument of the first call of the method publish() . This is
> clearly wrong.
>
> Two solutions :
>   - don't use an array in the signature of process() but use a List
> instead
>   - take the class of V in the constructor.
>     public SwingWorker(Class<T> tClass) {...}
>
> Note : i had reviewed the SwingWorker code when it was integrated to
> mustang (i think a year ago), it troubles me because
> it mix runtime type and generics but i was not able to produce a test
> case
> that show my insight until today.
> So the test case is not issue from a real application.
>
> Rémi Forax
>
> STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
> run the code
>
> EXPECTED VERSUS ACTUAL BEHAVIOR :
> EXPECTED -
> it should work
> ACTUAL -
> it throws a ClassCastException
>
> ERROR MESSAGES/STACK TRACES THAT OCCUR :
> Exception in thread "AWT-EventQueue-0" java.lang.ArrayStoreException
>     at java.lang.System.arraycopy(Native Method)
>     at java.util.ArrayList.toArray(ArrayList.java:306)
>     at
> sun.swing.AccumulativeRunnable.flush(AccumulativeRunnable.java:148)
>     at sun.swing.AccumulativeRunnable.run(AccumulativeRunnable.java:96)
>     at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:209)
>     at java.awt.EventQueue.dispatchEvent(EventQueue.java:598)
>     at
> java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:273)
>
>     at
> java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:183)
>
>     at
> java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:173)
>
>     at
> java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:168)
>     at
> java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:160)
>     at java.awt.EventDispatchThread.run(EventDispatchThread.java:121)
>
>
> REPRODUCIBILITY :
> This bug can be reproduced always.
>
> ---------- BEGIN SOURCE ----------
> import javax.swing.SwingWorker;
>
> public class SwingWorkerTest {
>     public static void main(String[] args) {
>       new SwingWorker<Void,CharSequence>() {
>         @Override
>         protected Void doInBackground() {
>           publish(new String[] {"hello"});
>           publish(new StringBuilder("world"));
>           return null;
>         }
>       }.execute();
>     }
> }
>
> ---------- END SOURCE ----------
>
> CUSTOMER SUBMITTED WORKAROUND :
> Add this line
> publish(new CharSequence[0]);
> as the first line of doInBackground().
> *** (#1 of 1): 2006-05-31 19:56:56 EDT *.*@sun.com



_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: SwingWorker a special producer/consumer

Joe Bowbeer
On 6/2/06, Rémi Forax <[hidden email]> wrote:
>
> The question is what is the best concurrent data structure
> for this job.
>

I'm looking at the source at https://swingworker.dev.java.net/

Is this sufficiently current?

If so, SwingWorker enqueues AccumulativeRunnable instances on the AWT
EventQueue by way of the SwingUtilities.invokeLater() method.

AccumulativeRunnable holds the accumulated arguments in an
ArrayList<T> and synchronizes internally.  So there's no need to pass
a thread-safe data structure to the process() method -- because
thread-safety is already handled by the SwingWorker internals.

I interpret the details as:

When the submitted runnable is executed on the event thread, it grabs
the existing elements from the internal ArrayList and calls the
run(T... args) method.  These elements are grabbed by the flush()
method.  The elements were added by the add() method.  Both methods
are synchronized.  In this way one or more process(V... chunks) calls
on the worker thread translate into a single run(T... args) call on
the event thread.

Are you proposing removing the synchronization from
AccumulativeRunnable and replacing it with some kind of thread-safe
collection?


On 6/2/06, Rémi Forax <[hidden email]> wrote:

> Pete Soper a écrit :
>
> > Hi Rémi,
> >     Nice to see your mail again. Are you looking for a suggestion for
> > a thread-safe queue or the like to pass stuff between your EDT and
> > your main thread?
> >
> > Regards,
> > Pete
>
> The bug reported is not a concurrency bug but
> because the implementation must be changed
> perhaps the signature of the process() method
> can be changed to take a thread safe List or a Queue.
>
> The SwingWorker is clearly a producer/consumer problem :
> - i want to put arrays of values (V)
> - i want to take all the values in one call.
> all in a thread safe way.
>
> The question is what is the best concurrent data structure
> for this job.
>
> Rémi Forax
>
> >
> > Gregg Wonderly wrote:
> >
> >>
> >> Rémi Forax wrote:
> >>
> >>> Yesterday, i found a bug in the Sun implementation of the SwingWorker :
> >>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6432565
> >>
> >>
> >>
> >> That bug is not visible yet, can you give specifics?
> >
> >
> > Still in dispatch:
> >
> > FULL PRODUCT VERSION :
> > java version "1.6.0-beta2"
> > Java(TM) SE Runtime Environment (build 1.6.0-beta2-b85)
> > Java HotSpot(TM) Client VM (build 1.6.0-beta2-b85, mixed mode, sharing)
> >
> >
> > ADDITIONAL OS VERSION INFORMATION :
> > Linux localhost.localdomain 2.6.15-1.1833_FC4 #1 Wed Mar 1 23:41:37
> > EST 2006 i686 i686 i386 GNU/Linux
> >
> >
> > A DESCRIPTION OF THE PROBLEM :
> > The EDT (Event dispatch Thread) raises a ClassCastException
> > with the code below.
> >
> > Data published by doInBackground() are stored in a list
> > and then converted in an array and send to method process()
> > in order to reduce synchronization time between EDT and worker thread.
> >
> > The problem is to determine the class of the array passed to the
> > process()
> > method at runtime. Because generics are erased, the class of V is not
> > available. The current implementation use the class of the array passed
> > as argument of the first call of the method publish() . This is
> > clearly wrong.
> >
> > Two solutions :
> >   - don't use an array in the signature of process() but use a List
> > instead
> >   - take the class of V in the constructor.
> >     public SwingWorker(Class<T> tClass) {...}
> >
> > Note : i had reviewed the SwingWorker code when it was integrated to
> > mustang (i think a year ago), it troubles me because
> > it mix runtime type and generics but i was not able to produce a test
> > case
> > that show my insight until today.
> > So the test case is not issue from a real application.
> >
> > Rémi Forax
> >
> > STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
> > run the code
> >
> > EXPECTED VERSUS ACTUAL BEHAVIOR :
> > EXPECTED -
> > it should work
> > ACTUAL -
> > it throws a ClassCastException
> >
> > ERROR MESSAGES/STACK TRACES THAT OCCUR :
> > Exception in thread "AWT-EventQueue-0" java.lang.ArrayStoreException
> >     at java.lang.System.arraycopy(Native Method)
> >     at java.util.ArrayList.toArray(ArrayList.java:306)
> >     at
> > sun.swing.AccumulativeRunnable.flush(AccumulativeRunnable.java:148)
> >     at sun.swing.AccumulativeRunnable.run(AccumulativeRunnable.java:96)
> >     at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:209)
> >     at java.awt.EventQueue.dispatchEvent(EventQueue.java:598)
> >     at
> > java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:273)
> >
> >     at
> > java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:183)
> >
> >     at
> > java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:173)
> >
> >     at
> > java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:168)
> >     at
> > java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:160)
> >     at java.awt.EventDispatchThread.run(EventDispatchThread.java:121)
> >
> >
> > REPRODUCIBILITY :
> > This bug can be reproduced always.
> >
> > ---------- BEGIN SOURCE ----------
> > import javax.swing.SwingWorker;
> >
> > public class SwingWorkerTest {
> >     public static void main(String[] args) {
> >       new SwingWorker<Void,CharSequence>() {
> >         @Override
> >         protected Void doInBackground() {
> >           publish(new String[] {"hello"});
> >           publish(new StringBuilder("world"));
> >           return null;
> >         }
> >       }.execute();
> >     }
> > }
> >
> > ---------- END SOURCE ----------
> >
> > CUSTOMER SUBMITTED WORKAROUND :
> > Add this line
> > publish(new CharSequence[0]);
> > as the first line of doInBackground().
> > *** (#1 of 1): 2006-05-31 19:56:56 EDT *.*@sun.com
>

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: SwingWorker a special producer/consumer

Remi Forax
Joe Bowbeer a écrit :

>On 6/2/06, Rémi Forax <[hidden email]> wrote:
>  
>
>>The question is what is the best concurrent data structure
>>for this job.
>>
>>    
>>
>
>I'm looking at the source at https://swingworker.dev.java.net/
>  
>
it seems to be the same code.

>Is this sufficiently current?
>  
>
>If so, SwingWorker enqueues AccumulativeRunnable instances on the AWT
>EventQueue by way of the SwingUtilities.invokeLater() method.
>
>AccumulativeRunnable holds the accumulated arguments in an
>ArrayList<T> and synchronizes internally.  So there's no need to pass
>a thread-safe data structure to the process() method -- because
>thread-safety is already handled by the SwingWorker internals.
>
>I interpret the details as:
>
>When the submitted runnable is executed on the event thread, it grabs
>the existing elements from the internal ArrayList and calls the
>run(T... args) method.  These elements are grabbed by the flush()
>method.  The elements were added by the add() method.  Both methods
>are synchronized.  In this way one or more process(V... chunks) calls
>on the worker thread translate into a single run(T... args) call on
>the event thread.
>  
>
yes

>Are you proposing removing the synchronization from
>AccumulativeRunnable and replacing it with some kind of thread-safe
>collection?
>  
>
Yes.
The bug is due to the fact that there is no way to generate an array of
T without
explicitly indicate the class of T.

One way to correct the bug is to change the signature of the run method
to run(List<T> args) or run(Queue<T> args).
In this case, its perhaps interresting to use a thread safe collection.

In perhaps it's a bad idea, i don't know, that why i ask for experts.

Rémi Forax



_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: SwingWorker a special producer/consumer

Joe Bowbeer
On 6/2/06, Rémi Forax <[hidden email]> wrote:
>
> The bug is due to the fact that there is no way to generate an array of
> T without explicitly indicate the class of T.
>
> One way to correct the bug is to change the signature of the run method
> to run(List<T> args) or run(Queue<T> args).
> In this case, its perhaps interresting to use a thread safe collection.
>

I think safe coding practices would dictate that the elements be
copied on their way into the AccumulativeRunnable from SwingWorker's
publish(V... chunks) method.

Likewise, I think safe coding practices would dictate that the
elements be copied on their way out of the AccumulativeRunnable into
SwingWorker's process(V... chunks) method.

The still leaves the question of whether AccumulativeRunnable
could/should use a concurrent collection instead of an externally
synchronized collection.

Attempting this in the most straightforward way would require that the
concurrent collection support atomic bulk operations such as
addAll(list) and drainTo(list), but the addAll method is not atomic
(AFAIK) and there is no drainTo method...

--Joe

On 6/2/06, Rémi Forax <[hidden email]> wrote:

> Joe Bowbeer a écrit :
>
> >On 6/2/06, Rémi Forax <[hidden email]> wrote:
> >
> >
> >>The question is what is the best concurrent data structure
> >>for this job.
> >>
> >>
> >>
> >
> >I'm looking at the source at https://swingworker.dev.java.net/
> >
> >
> it seems to be the same code.
>
> >Is this sufficiently current?
> >
> >
> >If so, SwingWorker enqueues AccumulativeRunnable instances on the AWT
> >EventQueue by way of the SwingUtilities.invokeLater() method.
> >
> >AccumulativeRunnable holds the accumulated arguments in an
> >ArrayList<T> and synchronizes internally.  So there's no need to pass
> >a thread-safe data structure to the process() method -- because
> >thread-safety is already handled by the SwingWorker internals.
> >
> >I interpret the details as:
> >
> >When the submitted runnable is executed on the event thread, it grabs
> >the existing elements from the internal ArrayList and calls the
> >run(T... args) method.  These elements are grabbed by the flush()
> >method.  The elements were added by the add() method.  Both methods
> >are synchronized.  In this way one or more process(V... chunks) calls
> >on the worker thread translate into a single run(T... args) call on
> >the event thread.
> >
> >
> yes
>
> >Are you proposing removing the synchronization from
> >AccumulativeRunnable and replacing it with some kind of thread-safe
> >collection?
> >
> >
> Yes.
> The bug is due to the fact that there is no way to generate an array of
> T without
> explicitly indicate the class of T.
>
> One way to correct the bug is to change the signature of the run method
> to run(List<T> args) or run(Queue<T> args).
> In this case, its perhaps interresting to use a thread safe collection.
>
> In perhaps it's a bad idea, i don't know, that why i ask for experts.
>
> Rémi Forax
>

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest