java.util.ConcurrentModificationException with Undo after multiple rapid swipe removals

[gwailoTr0n5000]

It seems that there may be a race condition or similar issue with multiple removals and then pressing "undo" on the snackbar if it coincides with the end of the timer before automated dismissal. I'm not able to easily replicate this but it has occurred about half a dozen times. It comes up most if I remove items quickly, then pause, then remove several more, then pause briefly, and click undo.

Error message:

05-25 12:52:41.784 21321-21321/com.SCME.GestureNotes E/SERVER_SYNC: outbound property change event: java.beans.PropertyChangeEvent[source=WebSocketManager{xxxxxxx #0 SERVER_DATA_SYNC}]
05-25 12:52:41.789 21321-21321/com.SCME.GestureNotes D/AndroidRuntime: Shutting down VM
05-25 12:52:41.789 21321-21321/com.SCME.GestureNotes E/AndroidRuntime: FATAL EXCEPTION: main
Process: com.SCME.GestureNotes, PID: 21321
java.util.ConcurrentModificationException
at java.util.TreeMap$MapIterator.stepForward(TreeMap.java:883)
at java.util.TreeMap$KeySet$1.next(TreeMap.java:959)
at eu.davidea.flexibleadapter.SelectableAdapter.clearSelection(SelectableAdapter.java:426)
at eu.davidea.flexibleadapter.FlexibleAdapter.clearSelection(FlexibleAdapter.java:470)
at eu.davidea.flexibleadapter.FlexibleAdapter.restoreDeletedItems(FlexibleAdapter.java:3736)
at com.SCME.GestureNotes.Fragment.SingleExpandableRecyclerViewListFragment.onUndoConfirmed(SingleExpandableRecyclerViewListFragment.java:438)
at eu.davidea.flexibleadapter.helpers.UndoHelper$1.onClick(UndoHelper.java:165)
at android.support.design.widget.Snackbar$1.onClick(Snackbar.java:255)
at android.view.View.performClick(View.java:5721)
at android.widget.TextView.performClick(TextView.java:10930)
at android.view.View$PerformClick.run(View.java:22620)
at android.os.Handler.handleCallback(Handler.java:739)
at android.os.Handler.dispatchMessage(Handler.java:95)
at android.os.Looper.loop(Looper.java:148)
at android.app.ActivityThread.main(ActivityThread.java:7331)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1230)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1120)

Relevant code:

	private FlexibleAdapter<IExpandableFlexibleState> mAdapter;

	@Override
	public void onUndoConfirmed(int action) {
		if (action == UndoHelper.ACTION_UPDATE) {
			//TODO: Complete click animation on swiped item
//			final RecyclerView.ViewHolder holder = mRecyclerView.findViewHolderForLayoutPosition(mSwipedPosition);
//			if (holder instanceof ItemTouchHelperCallback.ViewHolderCallback) {
//				final View view = ((ItemTouchHelperCallback.ViewHolderCallback) holder).getFrontView();
//				Animator animator = ObjectAnimator.ofFloat(view, "translationX", view.getTranslationX(), 0);
//				animator.addListener(new SimpleAnimatorListener() {
//					@Override
//					public void onAnimationCancel(Animator animation) {
//						view.setTranslationX(0);
//					}
//				});
//				animator.start();
//			}
		} else if (action == UndoHelper.ACTION_REMOVE) {
			// Custom action is restore deleted items
			**mAdapter.restoreDeletedItems();**
			// Disable Refreshing
//			mSwipeRefreshLayout.setRefreshing(false);
			// Check also selection restoration
			if (mAdapter.isRestoreWithSelection()) {
				// TODO: Implement the mActionModeHelper!!!!
				// TODO: Implement the mActionModeHelper!!!!
				// TODO: Implement the mActionModeHelper!!!!
//				mActionModeHelper.restoreSelection(this);
			}
		}
	}

I can wrap in a try/catch at the very least but wanted to bring it to your attention in case there is a more robust approach you would like to include in the library or in case I've just missed something.

Thank you very much for your help! This is an AWESOME library! It has been a pleasure to build on top of this.

[davideas]

@gwailoTr0n5000, thanks for compliments and to notifying about this issue.

Do you have the possibility to try the following fix inside the SelectableAdapter and with+without the fix in FlexibleAdapter and to reproduce it again?

public SelectableAdapter() {
	Log.i("FlexibleAdapter", "Running version " + BuildConfig.VERSION_NAME);
	// We synchronize the Set
	mSelectedPositions = Collections.synchronizedSet(new TreeSet<Integer>());
	....
}
public void restoreDeletedItems() {
	stopUndoTimer();
	multiRange = true;
	int initialCount = getItemCount();
	// Selection coherence: start from a clear situation
	// We avoid to clear selections if there aren't: swipe-to-delete hasn't (normally)
	if (getSelectedItemCount() > 0) clearSelection();
	...
}

[gwailoTr0n5000]

1. I have been using the SNAPSHOT build in my gradle file. Will this still be an acceptable test if I download the module for the latest RC or are there any conflicting changes in the snapshot?

2. Could you provide just a bit more context for where to place the lines of code?

I'm looking at these two spots (that I believe are the methods you're thinking of.

	/*--------------*/
	/* CONSTRUCTORS */
	/*--------------*/

	/**
	 * @since 1.0.0
	 */
	public SelectableAdapter() {
		Log.i("FlexibleAdapter", "Running version " + BuildConfig.VERSION_NAME);
		mSelectedPositions = new TreeSet<>();
		mBoundViewHolders = new HashSet<>();
		mMode = MODE_IDLE;

		mFastScrollerDelegate = new FastScroller.Delegate();
	}

--- and ---

	/**
	 * Restore items just removed.
	 * <p><b>Note:</b> If filter is active, only items that match that filter will be shown(restored).</p>
	 *
	 * @see #setRestoreSelectionOnUndo(boolean)
	 * @since 3.0.0
	 */
	@SuppressWarnings("ResourceType")
	public void restoreDeletedItems() {
		stopUndoTimer();
		multiRange = true;
		int initialCount = getItemCount();
		// Selection coherence: start from a clear situation
		clearSelection();
		// Start from latest item deleted, since others could rely on it
		for (int i = mRestoreList.size() - 1; i >= 0; i--) {
			adjustSelected = false;
			RestoreInfo restoreInfo = mRestoreList.get(i);

			if (restoreInfo.relativePosition >= 0) {
				// Restore child
				if (DEBUG) Log.d(TAG, "Restore SubItem " + restoreInfo);
				addSubItem(restoreInfo.getRestorePosition(true), restoreInfo.relativePosition,
						restoreInfo.item, false, Payload.UNDO);
			} else {
				// Restore parent or simple item
				if (DEBUG) Log.d(TAG, "Restore Item " + restoreInfo);
				addItem(restoreInfo.getRestorePosition(false), restoreInfo.item);
			}
			// Item is again visible
			restoreInfo.item.setHidden(false);
			// Notify header if exists
			IHeader header = getHeaderOf(restoreInfo.item);
			if (header != null) {
				notifyItemChanged(getGlobalPositionOf(header), Payload.UNDO);
			}
			// Restore header linkage
			if (unlinkOnRemoveHeader && isHeader(restoreInfo.item)) {
				header = (IHeader) restoreInfo.item;
				List<ISectionable> items = getSectionItems(header);
				for (ISectionable sectionable : items) {
					linkHeaderTo((T) sectionable, header, Payload.LINK);
				}
			}
		}
		// Restore selection if requested, before emptyBin
		if (restoreSelection && !mRestoreList.isEmpty()) {
			if (isExpandable(mRestoreList.get(0).item) || getExpandableOf(mRestoreList.get(0).item) == null) {
				parentSelected = true;
			} else {
				childSelected = true;
			}
			for (RestoreInfo restoreInfo : mRestoreList) {
				if (restoreInfo.item.isSelectable()) {
					addSelection(getGlobalPositionOf(restoreInfo.item));
				}
			}
			if (DEBUG) Log.d(TAG, "Selected positions after restore " + getSelectedPositions());
		}
		// Call listener to update EmptyView
		multiRange = false;
		if (mUpdateListener != null && initialCount == 0 && getItemCount() > 0)
			mUpdateListener.onUpdateEmptyView(getMainItemCount());

		emptyBin();
	}

[davideas]

The points are correct, but the current_snapshot branch has more commits than the published one because of a breaking change in createViewHolder() that won't make compile your code if I published (I reserve the publishing for the RC2, since I'm very close to do it).

If you check out this branch, you should reset the branch to this commit: d7c945a - 12/05/2017 at 00.01 -
Resolved 362 - Improved filter engine.