5

I am using a shared library in Java that returns ArrayList; as I iterate over it, a ConcurrentModificationException could be thrown and I am looking for 100% (?) guarantee to be safe. I was thinking on something like below and I'd appreciate any input.

The data_list is the ArrayList<> returned from the MT library.

boolean pass = true;

ArrayList<Something> local = new ArrayList<Something>(256);

for (int spin=0; spin<10; ++spin)
{
  try {
    local.addAll(data_list);
  }
  catch (java.util.ConcurrentModificationException ce) {
    pass = false;
  }
  finally {
    if (pass) break;
    pass = true;
  }
}

Assuming variable pass is true, how should I operate on local?

Colin D
  • 5,641
  • 1
  • 23
  • 35
Basixp
  • 115
  • 6
  • 5
    A multithreaded library exposes a non-concurrent collection that it keeps modifying after exposing? I think your best bet is to switch to another library. – Theodoros Chatzigiannakis Jul 01 '13 at 18:42
  • 2
    Not an answer to the question you've asked, but if you're copying one arraylist into another, it's generally more efficient to make the initial size of the destination the same as the source size, rather than a constant, i.e. `local = new ArrayList<>(data_list.size());` rather than just using 256. – Jules Jul 01 '13 at 19:05
  • I agree with @Theodoros, are you sure you are not trying to use the library in an incorrect manner? This seems like bad design of the library itself. – cjstehno Jul 01 '13 at 19:10
  • All of you are correct, BUT for whatever reason I must use this MT lib that exposes an unsafe ArrayList, and I am just trying to come up with some pattern that will at least diminish the probability for the concurrent exception; so what I am doing is iterating on the local copy while the exception can occur in the AddAll, I am "spinning" few times, which makes me believe I am in a better probability space; (but don't get me wrong, if I was a jet fighter pilot and this was one of the algorithms, I would not fly it ! – Basixp Jul 01 '13 at 19:12
  • @Basixp: you should tell us what library you are using, and how you are calling it. – Steven Schlansker Jul 01 '13 at 20:00

4 Answers4

9

There is no safe way to do this. You should not catch ConcurrentModificationException.

The iterators returned by this class's iterator and listIterator methods are fail-fast: if the list is structurally modified at any time after the iterator is created, in any way except through the iterator's own remove or add methods, the iterator will throw a ConcurrentModificationException. Thus, in the face of concurrent modification, the iterator fails quickly and cleanly, rather than risking arbitrary, non-deterministic behavior at an undetermined time in the future.

Note that the fail-fast behavior of an iterator cannot be guaranteed as it is, generally speaking, impossible to make any hard guarantees in the presence of unsynchronized concurrent modification. Fail-fast iterators throw ConcurrentModificationException on a best-effort basis. Therefore, it would be wrong to write a program that depended on this exception for its correctness: the fail-fast behavior of iterators should be used only to detect bugs.

Some collections, like HashMap, even can enter an infinite loop when used this way. Here's an explanation of how it happens.

You should not do this. There is no correct way to do this.

Either you misunderstand how the library works, or you need to switch out your library with one written by a competent developer.

What library are you using?

Community
  • 1
  • 1
Steven Schlansker
  • 37,580
  • 14
  • 81
  • 100
0

You don't define exactly what you mean by safe, and don't specify what kind of modifications are being performed to the list, but in many cases it may be acceptable to iterate over it manually by index, i.e.

for (int index = 0; index < data_list.size(); index ++)
    local.add(data_list.get(index));

The way I see it, there are four possible kinds of modification, with varying degrees of acceptability:

  • New items could be appended. This solution should work appropriately for this case, as long as the list does not grow enough to trigger a backing list expansion (and as this should happen with exponentially-reducing frequency, retrying if it occurs should be guaranteed to succeed eventually).
  • Existing items may be modified. This solution may not present a consistent view of the contents of the list at any given time, but it would be guaranteed to provide a usable list that is representative of items that have been in the list, which may be acceptable depending on your definition of "safe".
  • Items may be removed. There is a small chance this solution would fail with an IndexOutOfBoundsException, and the same caveat as for items being modified would apply with regards to consistency.
  • Items may be inserted into the middle of the list. The same caveat as items being modified would apply, and there would also be a danger of getting duplicated values. The problems with backing array expansion from the appending case would also apply.
Jules
  • 14,841
  • 9
  • 83
  • 130
0

You've got a bad situation here, but I think your solution is as sound as possible. The new ArrayList should go in the loop so you start fresh after each failure. Actually, the best thing might be to make your "try" line look like:

local = new ArrayList<Something>( data_list );

You don't want your ArrayList to have to expand itself because that will take time when you're trying to grab the data before the list changes. This should set the size, create it, and fill it with the least wasted effort.

You might need to catch things other than ConcurrentModification. You'll probably learn what the hard way. Or just catch Throwable.

If you want to go to extremes, run the code inside the for loop in it's own thread so if it does hang you can kill it and restart it. That's going to take some work.

I think this will work, if you let "spin" get large enough.

RalphChapin
  • 3,108
  • 16
  • 18
-2

I don't have any fundamental changes, but I think that code could be simplified a bit:

ArrayList<Something> local = new ArrayList<Something>(256);

for (int spin=0; spin<10; ++spin)
{
  try {
    local.addAll(data_list);
    break;
  }
  catch (java.util.ConcurrentModificationException ce) {}
}
raptortech97
  • 507
  • 3
  • 14