Hi Marco, yes I know it, what doesn't convince me is not the problem, but the solution. Have a look at Collections.* source code to see how this problem is fixed in synchronizedMap.
Anyway that confirms that maybe proxies are not the best to handle that situation - unless James has a solution (he's our proxy expert, just for the record) - and we have to manage read/write locks as James suggested - that it would be a nice addition -Simo http://people.apache.org/~simonetripodi/ http://simonetripodi.livejournal.com/ http://twitter.com/simonetripodi http://www.99soft.org/ On Sat, Mar 3, 2012 at 3:29 PM, Marco Speranza <marcospera...@apache.org> wrote: > 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 > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
