[
https://issues.apache.org/jira/browse/FINERACT-2787?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18108357#comment-18108357
]
Lea Fan commented on FINERACT-2787:
-----------------------------------
[~adamsaghy]Hi Adam,
I noticed that submitGSIMApplication creates the lock object inside the method.
As a result, each request uses a different lock, so the synchronized block does
not coordinate concurrent requests.
Within a single request, the for-loop already processes the client applications
sequentially, and @Transactional handles the database transaction.
Could you please confirm the intended behavior?
1. If the goal is only to process clients sequentially within one request, I
can remove the redundant synchronized block.
2. If concurrent submissions for the same group should be prevented, should the
coordination be scoped by groupId and implemented using a database or
transaction-level mechanism?
3. If all GSIM submissions should be serialized, should this also work across
multiple Fineract instances?
Once the intended behavior is confirmed, I can implement the appropriate change
and add a test.
Thanks!
> Ineffective method-local synchronization in GSIM application submission
> -----------------------------------------------------------------------
>
> Key: FINERACT-2787
> URL: https://issues.apache.org/jira/browse/FINERACT-2787
> Project: Apache Fineract
> Issue Type: Improvement
> Components: Savings
> Reporter: Lea Fan
> Priority: Minor
>
> Background
> The submitGSIMApplication method submits savings applications for the clients
> belonging to a GSIM group.
> Current implementation
> The method creates a new lock object for each invocation:
> final Object lock = new Object();
> synchronized (lock) {
> // submitApplication(...)
> }
> Problem
> Because the lock object is created inside the method, every invocation
> receives a different lock instance.
> Therefore, two concurrent requests executing submitGSIMApplication do not
> synchronize with each other. Each request locks only its own private object.
> Within a single invocation, the existing for-loop already invokes
> submitApplication sequentially, so the synchronized block does not provide
> additional ordering for that loop.
> This may give readers the misleading impression that concurrent GSIM
> application submissions are coordinated, although the lock cannot provide
> cross-request mutual exclusion.
> This issue does not claim that a user-visible concurrency failure has already
> been reproduced. The intended concurrency requirement should first be
> clarified.
> Expected behavior
> Clarify the intended concurrency semantics of GSIM application submission:
> * If cross-request mutual exclusion is not required, remove the ineffective
> local lock.
> * If cross-request coordination is required, use an appropriate shared,
> transactional, or database-level mechanism based on the business invariant
> being protected.
> Acceptance criteria
> * The method-local lock is removed or replaced with a mechanism that actually
> implements the intended concurrency behavior.
> * Tests cover the intended GSIM application submission behavior.
> * The implementation does not imply concurrency protection that it cannot
> provide.
> Related issues
> * FINERACT-603 - Original GSIM and GLIM implementation
> * FINERACT-2237 - GSIM parent account creation fix
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
