1

We've a controller which looks like below:

@RestController()
public class TestController {

  @Autowired
  TestService testService;

  ....

  // Update request
  @PutMapping("/update")
  public ResponseEntity<Sample> updateApi(@PathVariable(value = "id") Long id,
      @Valid @RequestBody Sample sampleDetails) {
    return new ResponseEntity<Sample>(testService.updateSample(id, sampleDetails), HttpStatus.OK);
  }

....

Service class(which has dependency on JPA repository) looks like:

@Service
public class TestService {

  @Autowired
  TestRepository testRepository;

  // update sample
  public Api updateSample(long sampleId, Sample details) {

    // Get object to be updated
    Sample sample = apiRepository.findById(sampleId);

    // Update required fields
    sample.setName(details.getName());
    sample.setBody(details.getBody());
    return apiRepository.save(api);
  }
  1. In the above service(TestService) setName and setBody method calls to jpa repository: Do these steps hold state(i.e not stateless) and can cause issue while handling concurrent requests. Or do we need to make scope of this service(or the updateSample method in service) as "web-aware Spring ApplicationContext"
  2. Similar to point 1, this line: "Sample sample = apiRepository.findById(sampleId);", Will this line be thread safe or can cause issue while handling concurrent requests.
Neo
  • 5,070
  • 10
  • 46
  • 65
  • Two things to consider: 1) database isolation level, locking 2) `@Transactional` - if you don't have that annotation on your method, those calls will be in different transactions. – Pijotrek Jan 25 '19 at 11:09
  • This code, is perfectly thread safe. As long as you don't keep state at the class level there is nothing wrong with it, – M. Deinum Jan 25 '19 at 12:37
  • don't keep state at the class level => does that mean that variables created inside the method are thread safe ? and only single thread will be executing the method at a given point of time ? – Neo Jan 25 '19 at 13:21
  • your bean is prototype or singleton? I thing the method is not "thread safe". Thread A and Thread B modify the same sampleId object,the result in database may not what you want. – TongChen Jan 25 '19 at 14:09
  • Its a singleton only – Neo Jan 25 '19 at 18:19

2 Answers2

1

You don't have any state which is shared between threads in your code. Unless your repository do not returns some cached object which could be shared between few thread.

If repository does DB call for every findById method invocation and creates new Sample object, the object is visible just for current thread. Why are local variables thread safe in Java

But you have Transaction Isolation problem because service's method updateSample executes out of transaction scope and data in DB can be changed by another thread or process between findById and save.

Vlad Bochenin
  • 3,007
  • 1
  • 20
  • 33
  • What should be best way to fix the thread issue in "updateSample": Should we make the scope as request aware or only the set of statement be made synchronised ? – Neo Jan 28 '19 at 06:12
  • @Neo You can mark the method with `@Transaction` annotation and use optimistic/pessimistic locking to solve concurrent modification issue https://www.baeldung.com/jpa-optimistic-locking – Vlad Bochenin Jan 28 '19 at 09:37
0

Well, let's go depth on how to work your example:

  1. Usually the application server (tomcat, undertown, jetty, others) manage a thread pool and every request is handled by a thread which is safe.
  2. When a user request enters in Controller, a thread safe is created and this keeps itself until the Controller returns a response in order to complete the request. So the request does some logic in Service and calls Repository will be in the thread safe.
  3. You can test this simple code with concurrent requests using some tool of stress (Gatling, JMeter, or other) and you won't have any issue due the application server creates a thread pool in order to manage threads safes that are handled concurrently, and so all shared resources they use (services, repositories etc.) must ensure thread safety.
Jonathan JOhx
  • 5,784
  • 2
  • 17
  • 33