The Read Committed isolation level ensures that data can only be read by a transaction if it is in the committed state. It doesn't completely isolate this transaction(create) hence some other transaction can still use this method which results in the behavior I explained previously. Ideally If we're gonna use @Transactional annotation the isolation level should be serialized for create operation. Please correct me If I'm wrong.
On Mon, Apr 9, 2018 at 1:12 PM, Francesco Chicchiriccò <[email protected]> wrote: > On 09/04/2018 09:30, Isuranga Perera wrote: > >> Hi Francesco, >> >> Yes there is @Transactional annotation. But it haven't set the isolation >> property as well as the propagation property. Based on the default values >> set this thread safe problem will still occur. Please correct me if I'm >> wrong. >> > > The transaction isolation level is set in > > https://github.com/apache/syncope/blob/master/core/persisten > ce-jpa/src/main/resources/domains/MasterDomain.xml#L57-L59 > > Regards. > > > On Mon, Apr 9, 2018 at 12:20 PM, Francesco Chicchiriccò < >> [email protected]> wrote: >> >> On 09/04/2018 08:46, Isuranga Perera wrote: >>> >>> Hi Francesco, >>>> >>>> I will take a scenario. Suppose a scenario where thread A & thread B try >>>> to login user admin. >>>> >>>> 1. thread A checks if a token exist for the user admin (suppose >>>> currently there is no token associated with the admin) >>>> 2. Then thread A execute following logic[1] to create and save the >>>> token. >>>> 3. Before thread A save the token for user admin thread B checks if a >>>> token exist for user admin (since the toked created by thread A is >>>> not yet saved *exist == null*) >>>> 4. Then thread A complete the creation of token (and saving) >>>> 5. Thread B also complete the creation and saving of the token. >>>> >>>> That way there can be 2 tokens for a particular user. >>>> >>>> >>>> You analysis does not take into any account the fact of the constraints >>> imposed by the @Service annotation in >>> >>> https://github.com/apache/syncope/blob/master/core/rest-cxf/ >>> src/main/java/org/apache/syncope/core/rest/cxf/service/Acces >>> sTokenServiceImpl.java#L35 >>> >>> (e.g. the place when external requests can come in) nor by the >>> @Transactional annotation injected into >>> >>> https://github.com/apache/syncope/blob/master/core/logic/ >>> src/main/java/org/apache/syncope/core/logic/AccessTokenLogic.java#L80 >>> >>> via >>> >>> https://github.com/apache/syncope/blob/master/core/logic/ >>> src/main/java/org/apache/syncope/core/logic/AbstractTra >>> nsactionalLogic.java#L29 >>> >>> Regards. >>> >>> [1] https://github.com/apache/syncope/blob/master/core/provision >>> >>>> ing-java/src/main/java/org/apache/syncope/core/provisioni >>>> ng/java/data/AccessTokenDataBinderImpl.java#L119 >>>> >>>> Best Regards >>>> Isuranga Perera >>>> >>>> On Mon, Apr 9, 2018 at 11:42 AM, Francesco Chicchiriccò < >>>> [email protected] <mailto:[email protected]>> wrote: >>>> >>>> On 09/04/2018 07:07, Isuranga Perera wrote: >>>> >>>> Hi All, >>>> >>>> Token create method in AccessTokenDataBinderImpl[1] is not >>>> thread safe. >>>> >>>> >>>> Could you please explain why you're affirming this? >>>> >>>> This could result in several problems including >>>> >>>> * Exist 2 different access token for a particular user at a >>>> given >>>> time which may result in an exception thrown by method >>>> call[2] >>>> since it expects a single token a given user. >>>> >>>> In addition to that token replace is implemented as a >>>> combination of 2 different functionalities. Since the method >>>> is not thread safe this may cause some unexpected behaviors >>>> (since there can be 2 tokens exist for a particular user. same >>>> scenario as above). >>>> >>>> Appreciate your insight on the $subject. >>>> >>>> >>>> [1] >>>> https://github.com/apache/syncope/blob/master/core/provision >>>> ing-java/src/main/java/org/apache/syncope/core/provisioni >>>> ng/java/data/AccessTokenDataBinderImpl.java#L104 >>>> <https://github.com/apache/syncope/blob/master/core/provisio >>>> ning-java/src/main/java/org/apache/syncope/core/provisioni >>>> ng/java/data/AccessTokenDataBinderImpl.java#L104> >>>> >>>> [2] >>>> https://github.com/apache/syncope/blob/master/core/provision >>>> ing-java/src/main/java/org/apache/syncope/core/provisioni >>>> ng/java/data/AccessTokenDataBinderImpl.java#L113 >>>> <https://github.com/apache/syncope/blob/master/core/provisio >>>> ning-java/src/main/java/org/apache/syncope/core/provisioni >>>> ng/java/data/AccessTokenDataBinderImpl.java#L113> >>>> >>>> Best Regards >>>> Isuranga Perera >>>> >>> > -- > Francesco Chicchiriccò > > Tirasa - Open Source Excellence > http://www.tirasa.net/ > > Member at The Apache Software Foundation > Syncope, Cocoon, Olingo, CXF, OpenJPA, PonyMail > http://home.apache.org/~ilgrosso/ > >
