How to avoid java.util.ConcurrentModificationException

Question

I'm using Hibernate in this application. I'm trying call data from database to jTable. When database is empty codes are compiling but when i add data to mysql table program throw java.util.ConcurrentModificationException.

 public FrmMain() {
    initialize();
    getData();
 }

 public void getData() {
    
    model = (DefaultTableModel) tblBookList.getModel();
    
    List<Book> books = new ArrayList<>(bookManager.getBook());
    
    ListIterator<Book> listIterator = books.listIterator();
      
    while(listIterator.hasNext()) {
      
      Book book = listIterator.next();
      
      Object[] row = { book.getName(), book.getAuthor(), book.getPublisher(),
      book.getPage(), book.getTranslator(), book.getPublishYear(), book.getType()
      };
      
      model.addRow(row);
      
    }
    
}

My data access codes. Maybe the problem is here

public class MySqlBookDal implements IBookDal {

SessionFactory factory = new Configuration()
        .configure("mysqlHibernate.cfg.xml")
        .addAnnotatedClass(Book.class)
        .buildSessionFactory();

Session session = factory.getCurrentSession(); 

public List<Book> select() {
    
    List<Book> books = new CopyOnWriteArrayList<Book>();
    
    try {
        
        session.beginTransaction();
        
        books = session.createQuery("from Book").getResultList();
        
        for(Book book: books) {
            books.add(book);
        }
        
        session.getTransaction().commit();
        
    } finally {
        factory.close();
    }
    
    return books;
}

Somebody can help me? Stack trace:

ERROR: Connection leak detected: there are 1 unclosed connections upon 

shutting down pool jdbc:mysql://localhost:3306/book?useUnicode=true&useLegacyDatetimeCode=false&serverTimezone=Turkey
java.util.ConcurrentModificationException
    at java.base/java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1013)
    at java.base/java.util.ArrayList$Itr.next(ArrayList.java:967)
    at com.github.bookManagementSystem.dataAccess.MySqlBookDal.select(MySqlBookDal.java:32)
    at com.github.bookManagementSystem.business.BookManager.getBook(BookManager.java:20)
    at com.github.bookManagementSystem.FrmMain.getData(FrmMain.java:98)
    at com.github.bookManagementSystem.FrmMain.<init>(FrmMain.java:90)
    at com.github.bookManagementSystem.FrmMain$1.run(FrmMain.java:76)
    at java.desktop/java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:316)
    at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:770)
    at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:721)
    at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:715)
    at java.base/java.security.AccessController.doPrivileged(AccessController.java:391)
    at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:85)
    at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:740)
    at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203)
    at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124)
    at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113)
    at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109)
    at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
    at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90)

What do you think this does? `for(Book book: books) { books.add(book); }` — Louis Wasserman, Nov 06 '20 at 07:07
@LouisWasserman I don't know why, I felt the need to put my list into a for loop. Thank You Louis — Tufan, Nov 06 '20 at 10:55
Here you're taking the result from query to the list which you created. books = session.createQuery("from Book").getResultList(); Instead create a new reference for it. Like List booksFromDb = ... then loop through it. — Sam, Nov 06 '20 at 12:49
Are you using Spring too? If so, this whole thing can be greatly simplified — riddle_me_this, Nov 06 '20 at 12:57
Why are you creating a `CopyOnWriteArrayList`? It is not needed at all — fps, Nov 06 '20 at 12:59
@fps I was trying a solution that was told to me but not work. — Tufan, Nov 06 '20 at 13:32

score 2 · Accepted Answer · answered Nov 06 '20 at 12:41

You are replacing the value of books with another list then the loop tries to append the new list books into itself - hence ConcurrentModificationException.

Assuming that getResultList() returns a List all you need to do is append results directly to books without re-assignment:

books.addAll(session.createQuery("from Book").getResultList());

// books = session.createQuery("from Book").getResultList();
// for(Book book: books) {
//         books.add(book);
// }

score 1 · Answer 2 · edited Nov 06 '20 at 13:09

1

Try using a CopyOnWriteArrayList instead of an ArrayList. This will allow for concurrent modification. But be warned, if you are changing the list a lot, this is not very efficient. If you are mostly reading, however, it should be fine.

edited Nov 06 '20 at 13:09

fps

33,623
8
55
110

answered Nov 05 '20 at 21:14

thetechnician94

545
2
21

@Tufan Can you add the entire stack trace to your question? Maybe the error is happening somewhere I wasnt expecting – thetechnician94 Nov 05 '20 at 21:31
i add stack trace, do you have any idea? I grateful for your helpfulness – Tufan Nov 05 '20 at 22:39
1

I see. Your issue is inside the method `bookManager.getBook()` and at `dataAccess.MySqlBookDal.select()` You'll need to add the code for that too – thetechnician94 Nov 05 '20 at 23:26
`CopyOnWriteArrayList` is NOT needed here. It is a very expensive data structure and the error of the OP has nothing to do with concurrency, despite the name of the exception. The error happens because they're adding books to the same list they are iterating – fps Nov 06 '20 at 12:59
1

@fps As I said, COWAL is only recommend if you are mostly reading from it. In this case it is not much more expensive than a regular ArrayList. If the issue is that they are adding books to the same list they are iterating, then that is a concurrency issue, by definition and could be solved with a COWAL, though restructuring the code might be a better approach. Further, when I wrote this answer we had far less information than we do now. – thetechnician94 Nov 06 '20 at 13:10
@thetechnician94 Yes, I realized there was much less information when you wrote your answer, that's why I retracted my downvote. Still, COWAL will throw the same exception and won't solve OP's problem. In Java, you can't structurally modify collections while iterating them, even from the same thread. That's the error here. It has nothing to do with different threads modifiying the list at the same time. Also, COWAL is very expensive, because the whole list is copied when every single book is added – fps Nov 06 '20 at 13:13

fps · Answer 3 · 2020-11-06T13:26:18.710

1

ConcurrentModificationException is being thrown because you are adding elements to the same list you are iterating. Here:

for(Book book : books) {
    books.add(book);
}

This code is wrong.

You can either return the List instance returned by Hibernate, or create a new list.

You don't need to use a CopyOnWriteArrayList! It is a very expensive datastructure. Your error has nothing to do with different threads modifiying and accessing the list at the same time. Besides, you don't need to start an explicit transaction against the database, because you are only reading. The whole select method could perfectly be simplified to:

public List<Book> select() {
    return new ArrayList<>(session.createQuery("from Book").getResultList());
}

You don't need to close the factory, either.

edited Nov 06 '20 at 13:26

answered Nov 06 '20 at 13:08

fps

33,623
8
55
110

ConcurrentModificationException does have everything to do with concurrency. Multiple threads are not required for definition concurrency issues. "Note that this exception does not always indicate that an object has been concurrently modified by a different thread. " https://docs.oracle.com/javase/7/docs/api/java/util/ConcurrentModificationException.html#:~:text=Class%20ConcurrentModificationException&text=This%20exception%20may%20be%20thrown,thread%20is%20iterating%20over%20it. – thetechnician94 Nov 06 '20 at 13:22
Also, for anyone who wants to know why `CopyOnWriteArrayList` is considered "expensive" https://stackoverflow.com/questions/17853112/in-what-situations-is-the-copyonwritearraylist-suitable#:~:text=Normally%20CopyOnWriteArrayList%20is%20very%20expensive,t%20modify%20it%20too%20often. – thetechnician94 Nov 06 '20 at 13:23
@thetechnician94 OK, I'll rephrase that in my answer, so that we don't use the word *concurrency* – fps Nov 06 '20 at 13:25
And I'll switch my vote to a +1. Sorry for any bluntness. – thetechnician94 Nov 06 '20 at 13:30
@fps I tried but not working, I encountered an error like: Calling method 'createQuery' is not valid without an active transaction (Current status: NOT_ACTIVE) – Tufan Nov 06 '20 at 13:43

score 0 · Answer 4 · answered Nov 06 '20 at 18:09

@DuncG has a solution to your immediate problem.

Your code may be simplified to something like the following with Spring Boot, assuming you download the needed dependencies through start.spring.io:

import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.stereotype.Repository;

@Repository
public interface BookRepository extends JpaRepository<Book, Long> {
}

In your service class:

@Autowired //or use constructor injection
BookRepository BookRepository;

public List<Book> findAll() {
    return bookRepository.findAll();
}

JpaRepository inherits the findAll() method as well as other very common behaviors with CRUD repositories. See the docs for further detail.

How to avoid java.util.ConcurrentModificationException

4 Answers4