Ciao
> rue, but *all* methods executions (synchronized and not) are > performed inside the handler, contained in the proxy instance anyway. > > So an user cannot use the same "lock" in order to synchronize a block > like this: Let's give you an example. here is our handler implementation, and imagine that the 'lock' is not export outside: === synchronized ( this.lock ) { try { return method.invoke( checkedToBeSynchronized, args ); } catch ( InvocationTargetException e ) { throw e.getTargetException(); } } === here is a possible user implementation: - Main class create a synchronized graph instance: MutableGraph g = CommonsGraph.synchronize( new UndirectMutableGraph.... ); - Thread 1 loops over the vertices: synchronized(g) { for ( BaseLabeledVertex v2 : g.getVertices() ) { // do somethings } } - Thread 2, insert a new vertex: for ( int i = start; i < end; i++ ) { BaseLabeledVertex v = new BaseLabeledVertex( valueOf( i ) ); g.addVertex( v ); } in that example thread, probably you would have a ConcurrentModificationException because: 1) Therad 1 uses a lock 'g' for the Iterator returned by g.getVertices() 2) Thread 2 uses its own lock 'this.lock' stored into Handler class. in this situation our data structure is *not* thread safe. The only way to synchronize the data structure is that: -Thread 2: for ( int i = start; i < end; i++ ) { synchronized(g) { BaseLabeledVertex v = new BaseLabeledVertex( valueOf( i ) ); g.addVertex( v ); } } but this IMHO is a wrong way to fix the problem, and moreover with this workaround we don't need to use CommonGraph.synchronize() method, isn't it? > I have not clear which benefit you want to obtain with that - looks at > Collections#synchronized* methods and see that the separation of > concerns in clear. you are right maybe use an external lock is not useful. > Sure that we can, the only issue we have is that GroboUtils is not in > the central repo, adding the external repository would make not easy > having the component released. unfortunately GroboUtil was the only lib that I found yesterday. Any suggestions to some other libs? all the best ;-) -- Marco Speranza <marcospera...@apache.org> Google Code: http://code.google.com/u/marco.speranza79/ Il giorno 03/mar/2012, alle ore 14:34, Simone Tripodi ha scritto: > Hola Marco, > >> my doubt is this: opening a synchronized block into handler implementation >> on 'this', in my opinion is not enough, because the method >> "CommonsGraph.synchronize()" returns a instance of the Proxy and not an >> instance of handler. > > true, but *all* methods executions (synchronized and not) are > performed inside the handler, contained in the proxy instance anyway. > > So an user cannot use the same "lock" in order to synchronize a block > like this: > > ==== > Graph g = CommonsGraph.synchronize(g_); > ... > synchronized(g) { > for ( BaseLabeledVertex v2 : g.getVertices() ) > { > // do somethings > } > } > ==== > > I am not sure we can do much more of what we've already done, for that > situation: the snippet below mixes the *data structure* > synchronization with the business logic performed outside. > >> So my idea is to open synchronized block inside handler implementation using >> the same Proxy instance that the method 'CommonsGraph.synchronize' will >> return. >> Furthermore I'd like to modify the API by adding also the possibility for >> the user to use an your own lock, in that way >> >> CommonsGraph.synchronize( G graph, Object lock ) > > I have not clear which benefit you want to obtain with that - looks at > Collections#synchronized* methods and see that the separation of > concerns in clear. > >> Finally only one question. I think that we should add some tests in order to >> check the correct implementation of our multithrading implementation. >> Do you think that we can use an external library in test scope? > > Sure that we can, the only issue we have is that GroboUtils is not in > the central repo, adding the external repository would make not easy > having the component released. > > best, > -Simo > > http://people.apache.org/~simonetripodi/ > http://simonetripodi.livejournal.com/ > http://twitter.com/simonetripodi > http://www.99soft.org/ > > > > On Sat, Mar 3, 2012 at 2:13 PM, Marco Speranza <marcospera...@apache.org> > wrote: >> Morning Simo, >> >> my doubt is this: opening a synchronized block into handler implementation >> on 'this', in my opinion is not enough, because the method >> "CommonsGraph.synchronize()" returns a instance of the Proxy and not an >> instance of handler. >> So an user cannot use the same "lock" in order to synchronize a block like >> this: >> >> ==== >> Graph g = CommonsGraph.synchronize(g_); >> ... >> synchronized(g) { >> for ( BaseLabeledVertex v2 : g.getVertices() ) >> { >> // do somethings >> } >> } >> ==== >> >> So my idea is to open synchronized block inside handler implementation using >> the same Proxy instance that the method 'CommonsGraph.synchronize' will >> return. >> Furthermore I'd like to modify the API by adding also the possibility for >> the user to use an your own lock, in that way >> >> CommonsGraph.synchronize( G graph, Object lock ) >> >> WDYT? >> >> Finally only one question. I think that we should add some tests in order to >> check the correct implementation of our multithrading implementation. >> Do you think that we can use an external library in test scope? >> >> >> have a nice day >> >> >> -- >> Marco Speranza <marcospera...@apache.org> >> Google Code: http://code.google.com/u/marco.speranza79/ >> >> Il giorno 03/mar/2012, alle ore 13:50, Simone Tripodi ha scritto: >> >>> Good morning Marco, >>> >>> I had the chance to have a deeper look at your yesterday's night work >>> and think your additions are good improvements - I just wonder if we >>> can replace the lock object with the handler itself, referencing >>> `this` instead. >>> >>> Thoughts? >>> >>> best and have a nice WE, >>> -Simo >>> >>> http://people.apache.org/~simonetripodi/ >>> http://simonetripodi.livejournal.com/ >>> http://twitter.com/simonetripodi >>> http://www.99soft.org/ >>> >>> >>> >>> On Sat, Mar 3, 2012 at 1:56 AM, Simone Tripodi <simonetrip...@apache.org> >>> wrote: >>>> I saw the commit, please read comments inline. >>>> best, >>>> -Simo >>>> >>>> http://people.apache.org/~simonetripodi/ >>>> http://simonetripodi.livejournal.com/ >>>> http://twitter.com/simonetripodi >>>> http://www.99soft.org/ >>>> >>>> >>>> >>>> On Sat, Mar 3, 2012 at 1:40 AM, Marco Speranza <marcospera...@apache.org> >>>> wrote: >>>>> Hi I fixed the problem using proxy/handler. >>>>> >>>>> I put also a couple of tests. For do that I insert a test scoped >>>>> dependency to a library net.sourceforge.groboutils.groboutils-core >>>>> >>>>> Ciao >>>>> >>>>> -- >>>>> Marco Speranza <marcospera...@apache.org> >>>>> Google Code: http://code.google.com/u/marco.speranza79/ >>>>> >>>>> Il giorno 03/mar/2012, alle ore 01:29, Simone Tripodi ha scritto: >>>>> >>>>>> yes, what I didn't understand is how you would like to manage the >>>>>> iterators issue >>>>>> >>>>>> sorry for the brevity but tonight I am getting crazy with at least 3 >>>>>> other projects :D >>>>>> >>>>>> -Simo >>>>>> >>>>>> http://people.apache.org/~simonetripodi/ >>>>>> http://simonetripodi.livejournal.com/ >>>>>> http://twitter.com/simonetripodi >>>>>> http://www.99soft.org/ >>>>>> >>>>>> >>>>>> >>>>>> On Sat, Mar 3, 2012 at 1:16 AM, Marco Speranza >>>>>> <marcospera...@apache.org> wrote: >>>>>>> I think that we have to use the same patter of java Collections: a >>>>>>> wrapper of Graph/MutableGraph that use a synchronize block. >>>>>>> >>>>>>> WDYT? >>>>>>> >>>>>>> -- >>>>>>> Marco Speranza <marcospera...@apache.org> >>>>>>> Google Code: http://code.google.com/u/marco.speranza79/ >>>>>>> >>>>>>> Il giorno 03/mar/2012, alle ore 01:10, Simone Tripodi ha scritto: >>>>>>> >>>>>>>> OK now sounds better, thanks - how would you intend to fix it? >>>>>>>> TIA, >>>>>>>> -Simo >>>>>>>> >>>>>>>> http://people.apache.org/~simonetripodi/ >>>>>>>> http://simonetripodi.livejournal.com/ >>>>>>>> http://twitter.com/simonetripodi >>>>>>>> http://www.99soft.org/ >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Sat, Mar 3, 2012 at 1:01 AM, Marco Speranza >>>>>>>> <marcospera...@apache.org> wrote: >>>>>>>>>>> >>>>>>>>>>> furthermore there is another problem: with handler is not possible >>>>>>>>>>> to use correctly synchronization block like this >>>>>>>>>>> >>>>>>>>>>> ==== >>>>>>>>>>> Graph g = CommonsGraph.synchronize(g_); >>>>>>>>>>> ... >>>>>>>>>>> synchronized(g) { >>>>>>>>>>> for ( BaseLabeledVertex v2 : g.getVertices() ) >>>>>>>>>>> { >>>>>>>>>>> // do somethings >>>>>>>>>>> } >>>>>>>>>>> } >>>>>>>>>>> ==== >>>>>>>>>> >>>>>>>>>> sorry I don't understand what you meant/intend to do >>>>>>>>> >>>>>>>>> I'm trying to explain better. the method getVertices return an >>>>>>>>> Iterator. In a multi-thread environment you have to ensure that >>>>>>>>> during the 'for' execution the [graph] data structure remains >>>>>>>>> consistent. >>>>>>>>> Also for java collection is the same >>>>>>>>> [http://docs.oracle.com/javase/6/docs/api/java/util/Collections.html#synchronizedCollection(java.util.Collection)] >>>>>>>>> >>>>>>>>> So if we use a proxy/handler there is no way to "export" the >>>>>>>>> mutex/lock variable used into handler class. >>>>>>>>> >>>>>>>>> In our CommonsGraph the variable this.lock is private and is non >>>>>>>>> possible to export out of the method, so the user can not utilize the >>>>>>>>> lock to synchronize his block. >>>>>>>>> >>>>>>>>> === >>>>>>>>> public Object invoke( Object proxy, Method method, Object[] >>>>>>>>> args ) >>>>>>>>> throws Throwable >>>>>>>>> { >>>>>>>>> if ( synchronizedMethods.contains( method ) ) >>>>>>>>> { >>>>>>>>> try >>>>>>>>> { >>>>>>>>> synchronized ( this.lock ) >>>>>>>>> { >>>>>>>>> return method.invoke( synchronizedMethods, >>>>>>>>> args ); >>>>>>>>> } >>>>>>>>> } >>>>>>>>> catch ( InvocationTargetException e ) >>>>>>>>> { >>>>>>>>> throw e.getTargetException(); >>>>>>>>> } >>>>>>>>> } >>>>>>>>> return method.invoke( synchronizedMethods, args ); >>>>>>>>> } >>>>>>>>> === >>>>>>>>> >>>>>>>>> ciao >>>>>>>>> >>>>>>>>> -- >>>>>>>>> Marco Speranza <marcospera...@apache.org> >>>>>>>>> Google Code: http://code.google.com/u/marco.speranza79/ >>>>>>>>> >>>>>>>>> Il giorno 03/mar/2012, alle ore 00:45, Simone Tripodi ha scritto: >>>>>>>>> >>>>>>>>>>> furthermore there is another problem: with handler is not possible >>>>>>>>>>> to use correctly synchronization block like this >>>>>>>>>>> >>>>>>>>>>> ==== >>>>>>>>>>> Graph g = CommonsGraph.synchronize(g_); >>>>>>>>>>> ... >>>>>>>>>>> synchronized(g) { >>>>>>>>>>> for ( BaseLabeledVertex v2 : g.getVertices() ) >>>>>>>>>>> { >>>>>>>>>>> // do somethings >>>>>>>>>>> } >>>>>>>>>>> } >>>>>>>>>>> ==== >>>>>>>>>> >>>>>>>>>> sorry I don't understand what you meant/intend to do >>>>>>>>>> >>>>>>>>>>> I really think that a synchronized wrapper it's the best solution. >>>>>>>>>>> >>>>>>>>>>> WDYT? >>>>>>>>>> >>>>>>>>>> they sucks because we have to keep them updated while , but if there >>>>>>>>>> are not alternatives... >>>>>>>>>> -Simo >>>>>>>>>> >>>>>>>>>> http://people.apache.org/~simonetripodi/ >>>>>>>>>> http://simonetripodi.livejournal.com/ >>>>>>>>>> http://twitter.com/simonetripodi >>>>>>>>>> http://www.99soft.org/ >>>>>>>>>> >>>>>>>>>> --------------------------------------------------------------------- >>>>>>>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>>>>>>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> --------------------------------------------------------------------- >>>>>>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>>>>>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>>>>>>> >>>>>>>> >>>>>>>> --------------------------------------------------------------------- >>>>>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>>>>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>>>>>> >>>>>>> >>>>>>> >>>>>>> --------------------------------------------------------------------- >>>>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>>>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>>>>> >>>>>> >>>>>> --------------------------------------------------------------------- >>>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>>>> >>>>> >>>>> >>>>> --------------------------------------------------------------------- >>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>>> >>> >>> --------------------------------------------------------------------- >>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>> For additional commands, e-mail: dev-h...@commons.apache.org >>> >> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> For additional commands, e-mail: dev-h...@commons.apache.org >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org