Skip to content
This repository has been archived by the owner on Oct 30, 2023. It is now read-only.

Add guards around structures to prevent racing. #74

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add guards around structures to prevent racing. #74

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eccf171
Add guards around structures to prevent racing.
antihax Mar 17, 2017
4510cb1
rework
antihax Mar 17, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 49 additions & 38 deletions java/java-jersey-jaxrs/src/main/java/io/swagger/sample/data/PetData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@
import java.util.*;

public class PetData {
static List<Pet> pets = new ArrayList<Pet>();
static List<Category> categories = new ArrayList<Category>();

static List<Pet> pets = Collections.synchronizedList(new ArrayList<Pet>());
static List<Category> categories = Collections.synchronizedList(new ArrayList<Category>());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you don't need to synchronize this, as it is not used anywhere outside the static initializer.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed. Thanks.


static {
categories.add(createCategory(1, "Dogs"));
Expand Down Expand Up @@ -68,11 +69,13 @@ public Pet getPetById(long petId) {

Copy link

@ePaul ePaul Mar 17, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you in almost all methods synchronize on pets, just not in getPetById (some lines up from here). Intentionally?
I'm not sure if this here could create some issue ...

The iterator is not synchronized, but checks for concurrent modifications (in an unsynchronized way).
So if any other request gets in between with deletion/addition, this could throw an exception.

On the other hand, if you synchronize everything anyways, you could get rid of the synchronizedList.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed the iterator around it. I also removed the synchronizedLists since all the operations are within sync {}.

public boolean deletePet(long petId) {
if(pets.size() > 0) {
for (int i = pets.size() - 1; i >= 0; i--) {
Pet pet = pets.get(i);
if(pet.getId() == petId) {
pets.remove(i);
return true;
synchronized (pets) {
for (int i = pets.size() - 1; i >= 0; i--) {
Pet pet = pets.get(i);
if(pet.getId() == petId) {
pets.remove(i);
return true;
}
}
}
}
Expand All @@ -85,10 +88,12 @@ public List<Pet> findPetByStatus(String status) {
return result;
}
String[] statuses = status.split(",");
for (Pet pet : pets) {
for (String s : statuses) {
if (s.equals(pet.getStatus())) {
result.add(pet);
synchronized (pets) {
for (Pet pet : pets) {
for (String s : statuses) {
if (s.equals(pet.getStatus())) {
result.add(pet);
}
}
}
}
Expand All @@ -102,12 +107,14 @@ public List<Pet> findPetByTags(String tags) {
return result;
}
String[] tagList = tags.split(",");
for (Pet pet : pets) {
if (null != pet.getTags()) {
for (Tag tag : pet.getTags()) {
for (String tagListString : tagList) {
if (tagListString.equals(tag.getName()))
result.add(pet);
synchronized (pets) {
for (Pet pet : pets) {
if (null != pet.getTags()) {
for (Tag tag : pet.getTags()) {
for (String tagListString : tagList) {
if (tagListString.equals(tag.getName()))
result.add(pet);
}
}
}
}
Expand All @@ -116,37 +123,41 @@ public List<Pet> findPetByTags(String tags) {
}

public Pet addPet(Pet pet) {
if(pet.getId() == 0) {
long maxId = 0;
for (int i = pets.size() - 1; i >= 0; i--) {
if(pets.get(i).getId() > maxId) {
maxId = pets.get(i).getId();
synchronized (pets) {
if(pet.getId() == 0) {
long maxId = 0;
for (int i = pets.size() - 1; i >= 0; i--) {
if(pets.get(i).getId() > maxId) {
maxId = pets.get(i).getId();
}
}
pet.setId(maxId + 1);
}
pet.setId(maxId + 1);
}
if (pets.size() > 0) {
for (int i = pets.size() - 1; i >= 0; i--) {
if (pets.get(i).getId() == pet.getId()) {
pets.remove(i);
if (pets.size() > 0) {
for (int i = pets.size() - 1; i >= 0; i--) {
if (pets.get(i).getId() == pet.getId()) {
pets.remove(i);
}
}
}
pets.add(pet);
}
pets.add(pet);
return pet;
}

public Map<String, Integer> getInventoryByStatus() {
Map<String, Integer> output = new HashMap<String, Integer>();
for(Pet pet : pets) {
String status = pet.getStatus();
if(status != null && !"".equals(status)) {
Integer count = output.get(status);
if(count == null)
count = new Integer(1);
else
count = count.intValue() + 1;
output.put(status, count);
synchronized (pets) {
for(Pet pet : pets) {
String status = pet.getStatus();
if(status != null && !"".equals(status)) {
Integer count = output.get(status);
if(count == null)
count = new Integer(1);
else
count = count.intValue() + 1;
output.put(status, count);
}
}
}
return output;
Expand Down
40 changes: 22 additions & 18 deletions java/java-jersey-jaxrs/src/main/java/io/swagger/sample/data/StoreData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,10 @@

import io.swagger.sample.model.Order;

import java.util.Date;
import java.util.List;
import java.util.ArrayList;
import java.util.*;

public class StoreData {
static List<Order> orders = new ArrayList<Order>();
static List<Order> orders = Collections.synchronizedList(new ArrayList<Order>());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you always access this in a synchronized block, there is no need for the additional synchronizedList.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed. Thanks,


static {
orders.add(createOrder(1, 1, 2, new Date(), "placed"));
Expand All @@ -39,32 +37,38 @@ public class StoreData {
}

public Order findOrderById(long orderId) {
for (Order order : orders) {
if (order.getId() == orderId) {
return order;
synchronized (orders) {
for (Order order : orders) {
if (order.getId() == orderId) {
return order;
}
}
}
return null;
}

public Order placeOrder(Order order) {
if (orders.size() > 0) {
for (int i = orders.size() - 1; i >= 0; i--) {
if (orders.get(i).getId() == order.getId()) {
orders.remove(i);
}
synchronized (orders) {
if (orders.size() > 0) {
for (int i = orders.size() - 1; i >= 0; i--) {
if (orders.get(i).getId() == order.getId()) {
orders.remove(i);
}
}
orders.add(order);
}
return order;
}
orders.add(order);
return order;
}

public boolean deleteOrder(long orderId) {
if (orders.size() > 0) {
for (int i = orders.size() - 1; i >= 0; i--) {
if (orders.get(i).getId() == orderId) {
orders.remove(i);
return true;
synchronized (orders) {
for (int i = orders.size() - 1; i >= 0; i--) {
if (orders.get(i).getId() == orderId) {
orders.remove(i);
return true;
}
}
}
}
Expand Down
36 changes: 21 additions & 15 deletions java/java-jersey-jaxrs/src/main/java/io/swagger/sample/data/UserData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,10 @@

import io.swagger.sample.model.User;

import java.util.List;
import java.util.ArrayList;
import java.util.*;

public class UserData {
static List<User> users = new ArrayList<User>();
static List<User> users = Collections.synchronizedList( new ArrayList<User>() );

static {
users.add(createUser(1, "user1", "first name 1", "last name 1",
Expand Down Expand Up @@ -51,9 +50,11 @@ public class UserData {
}

public User findUserByName(String username) {
for (User user : users) {
if (user.getUsername().equals(username)) {
return user;
synchronized (users) {
for (User user : users) {
if (user.getUsername().equals(username)) {
return user;
}
}
}
return null;
Expand All @@ -62,22 +63,27 @@ public User findUserByName(String username) {
public void addUser(User user) {
if(user.getUsername() == null)
return;
if (users.size() > 0) {
for (int i = users.size() - 1; i >= 0; i--) {
if (users.get(i).getUsername().equals(user.getUsername())) {
users.remove(i);

synchronized (users) {
if (users.size() > 0) {
for (int i = users.size() - 1; i >= 0; i--) {
if (users.get(i).getUsername().equals(user.getUsername())) {
users.remove(i);
}
}
}
users.add(user);
}
users.add(user);
}

public boolean removeUser(String username) {
if (users.size() > 0) {
for (int i = users.size() - 1; i >= 0; i--) {
if (users.get(i).getUsername().equals(username)) {
users.remove(i);
return true;
synchronized (users) {
for (int i = users.size() - 1; i >= 0; i--) {
if (users.get(i).getUsername().equals(username)) {
users.remove(i);
return true;
}
}
}
}
Expand Down