ArrayNew is not default synchronized=true and missing typed arrays documentation

Description

The documentation claims the ArrayNew() synchronized option is defaulting to true, but it isn't anymore. Arrays also historically used to be synchronized in Lucee 4.5 and maybe prior versions too.
https://docs.lucee.org/reference/functions/arraynew.html

I looked at the implementation, and I see ArrayList is used by default with no synchronization as the underlying data type. Only arrays with a larger dimension are synchronized in ArrayClassic, which used to be the only kind of Array.

Also, are you sure you want the new type argument to be in between dimension and synchronized? Everyone using that on ACF and coming to Lucee would have to rewrite those manually to get compatible. Easier to make that argument #3 instead. The documentation for this brand new feature is currently missing as well.

It is probably better to make synchronization the default so a developer chooses when to incur this risk since thread safety bugs are so confusing and random when they come up.

I noticed this because ArraySort is able to throw concurrentModificationException in my real application. I duplicated the array to fix that. If the CFML developer was being told to synchronize arrays themselves, the documentation should be updated to warn them the default was changed in X version.

I see Vector commented out. I read about Vector, which is synchronized unlike ArrayList. It sounds like Vector is discouraged because individually locking every array operation is inefficient compared to 0 or 1 lock around the group of operations. It seems like you have Lucee setup the fastest performing way now, but would need to announce the behavior change to get people ready for it. Maybe the Lucee admin option route is an easier way to handle it globally since those seeking performance, could be willing to change the default behavior. This is a break in compatibility with both ACF and older Lucee so it is currently behaving like a bug.

Because ArrayImpl.java is used for lots of internal objects that don't seem to require thread safety it seems like ArrayImpl.java should stay the same. A new class is better, or we do nothing and let people fix their own Concurrency errors individually. It will give the impression Lucee is not stable if one brought in code from ACF that was thread-safe there, and now isn't. It would protect Lucee reputation to give the Lucee admin treatment to this idea, instead of receiving more forum/bug reports on it.

To make it easier for developers to understand thread safety issues, maybe it would help if you augmented ConcurrentModificationException error reporting with a better description of the problem when you can detect it happened on a CFML object. A better error could say "Write operations to this type of array are not thread-safe, and a ConcurrentModificationException was thrown, meaning another thread attempt to access the array while it was being modified. You must create a synchronized array (see ArrayNew documentation), add a lock around the array or create a duplicate() of the array to avoid this error." and maybe show the array variable name with reflection.

Looks like a quick fix in ArrayUtil.java getInstance() if we wanted to change it back or make an option for it.

Environment

None

Status

Assignee

Michael Offner

Reporter

Bruce Kirkpatrick

Labels

None

Sprint

None

Fix versions

Affects versions

5.3.2.16

Priority

Critical
Configure