You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fory.apache.org by "vybhavjs (via GitHub)" <gi...@apache.org> on 2025/12/23 13:58:32 UTC
[PR] fix(java): Fix nested CopyOnWriteArrayList serialization [fory]
vybhavjs opened a new pull request, #3079:
URL: https://github.com/apache/fory/pull/3079
## Why?
Fory doesn't currently support serialization of any object that has a `CopyOnWriteArrayList` field with codegen, because the generated code attempts to cast `CollectionSnapshot` to `List`, throwing the following exception :
```
org.apache.fory.exception.SerializationException: java.lang.ClassCastException:
class org.apache.fory.collection.CollectionSnapshot cannot be cast to class java.util.List
(org.apache.fory.collection.CollectionSnapshot is in unnamed module of loader 'app';
java.util.List is in module java.base of loader 'bootstrap')
org.apache.fory.serializer.collection.CollectionSerializersTest.testCopyOnWriteArrayListNested(CollectionSerializersTest.java:407)
Caused by: java.lang.ClassCastException: class org.apache.fory.collection.CollectionSnapshot cannot be cast to class java.util.List
(org.apache.fory.collection.CollectionSnapshot is in unnamed module of loader 'app';
java.util.List is in module java.base of loader 'bootstrap')
at org.apache.fory.serializer.collection.CollectionSerializersTest_NestedCopyOnWriteArrayListForyCodec_0.writeFields$(CollectionSerializersTest_NestedCopyOnWriteArrayListForyCodec_0.java:63)
at org.apache.fory.serializer.collection.CollectionSerializersTest_NestedCopyOnWriteArrayListForyCodec_0.write(CollectionSerializersTest_NestedCopyOnWriteArrayListForyCodec_0.java:130)
```
This is the relevant code from the the generated codec that attempts the invalid class cast.
```
CollectionLikeSerializer collectionLikeSerializer = this.writeCollectionClassInfo(list0, memoryBuffer1);
if (collectionLikeSerializer.supportCodegenHook()) {
java.util.List list1 = (java.util.List)collectionLikeSerializer.onCollectionWrite(memoryBuffer1, list0);
```
## What does this PR do?
* feat: Define `ListSnapshot extends AbstractList` similar to existing `CollectionSnapshot` and `MapSnapshot`.
* feat: Define `ConcurrentListSerializer`, similar to existing `ConcurrentCollectionSerializer` and `ConcurrentMapSerializer`
* fix: Update `CopyOnWriteArrayListSerializer` to extend the new `ConcurrentListSerializer`
* test: Add `ListSnapshotTest` to test `ListSnapshot` behaviour
* test: Add test in `CollectionSerializersTest` to test nested CopyOnWriteArrayList serialization
## Related issues
Fixes #2918.
## Does this PR introduce any user-facing change?
<!--
If any user-facing interface changes, please [open an issue](https://github.com/apache/fory/issues/new/choose) describing the need to do so and update the document if necessary.
Delete section if not applicable.
-->
- [ ] Does this PR introduce any public API change?
- [ ] Does this PR introduce any binary protocol compatibility change?
## Benchmark
<!--
When the PR has an impact on performance (if you don't know whether the PR will have an impact on performance, you can submit the PR first, and if it will have impact on performance, the code reviewer will explain it), be sure to attach a benchmark data here.
Delete section if not applicable.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(java): Fix CopyOnWriteArrayList field serialization [fory]
Posted by "vybhavjs (via GitHub)" <gi...@apache.org>.
vybhavjs commented on code in PR #3079:
URL: https://github.com/apache/fory/pull/3079#discussion_r2643547538
##########
java/fory-core/src/main/java/org/apache/fory/collection/ListSnapshot.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fory.collection;
+
+import java.util.AbstractList;
+import java.util.Iterator;
+import java.util.List;
+import org.apache.fory.annotation.Internal;
+
+/**
+ * A specialized list implementation that creates a snapshot of elements for iteration with indexed
+ * access support. This class is designed to efficiently handle concurrent list serialization by
+ * creating a lightweight snapshot that can be iterated without holding references to the original
+ * list elements.
+ *
+ * <p>The implementation uses an array-based storage for entries and provides optimized iteration
+ * through a custom iterator. It includes memory management features such as automatic array
+ * reallocation when clearing large collections to prevent memory leaks.
+ *
+ * <p><strong>Important:</strong> The returned iterator from {@link #iterator()} must be consumed
+ * completely before calling {@code iterator()} again. The iterator maintains its position through a
+ * shared index, and calling {@code iterator()} again before the previous iterator is fully consumed
+ * will result in incorrect iteration behavior.
+ *
+ * <p>This class is marked as {@code @Internal} and should not be used directly by application code.
+ * It's specifically designed for internal serialization purposes.
+ *
+ * @param <E> the type of elements maintained by this list
+ * @since 1.0
+ */
+@Internal
+public class ListSnapshot<E> extends AbstractList<E> {
Review Comment:
That was my initial thought as most of the code is duplicated, and implementing `get(index)` is possible even with `CollectionSnapshot`. I decided against it because It seemed a bit odd semantically as now, non-list classes like `CopyOnWriteArraySet`, `ConcurrentSkipListSet` etc are being wrapped in a List implementation.
**All tests are pass with that change though** - so should I go with that approach instead?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(java): Fix CopyOnWriteArrayList field serialization [fory]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #3079:
URL: https://github.com/apache/fory/pull/3079#discussion_r2643443459
##########
java/fory-core/src/main/java/org/apache/fory/collection/ListSnapshot.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fory.collection;
+
+import java.util.AbstractList;
+import java.util.Iterator;
+import java.util.List;
+import org.apache.fory.annotation.Internal;
+
+/**
+ * A specialized list implementation that creates a snapshot of elements for iteration with indexed
+ * access support. This class is designed to efficiently handle concurrent list serialization by
+ * creating a lightweight snapshot that can be iterated without holding references to the original
+ * list elements.
+ *
+ * <p>The implementation uses an array-based storage for entries and provides optimized iteration
+ * through a custom iterator. It includes memory management features such as automatic array
+ * reallocation when clearing large collections to prevent memory leaks.
+ *
+ * <p><strong>Important:</strong> The returned iterator from {@link #iterator()} must be consumed
+ * completely before calling {@code iterator()} again. The iterator maintains its position through a
+ * shared index, and calling {@code iterator()} again before the previous iterator is fully consumed
+ * will result in incorrect iteration behavior.
+ *
+ * <p>This class is marked as {@code @Internal} and should not be used directly by application code.
+ * It's specifically designed for internal serialization purposes.
+ *
+ * @param <E> the type of elements maintained by this list
+ * @since 1.0
+ */
+@Internal
+public class ListSnapshot<E> extends AbstractList<E> {
Review Comment:
Could we remove CollectionSnapshot and use ListSnapshot acroos whol codebase?
##########
java/fory-core/src/main/java/org/apache/fory/collection/ListSnapshot.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fory.collection;
+
+import java.util.AbstractList;
+import java.util.Iterator;
+import java.util.List;
+import org.apache.fory.annotation.Internal;
+
+/**
+ * A specialized list implementation that creates a snapshot of elements for iteration with indexed
+ * access support. This class is designed to efficiently handle concurrent list serialization by
+ * creating a lightweight snapshot that can be iterated without holding references to the original
+ * list elements.
+ *
+ * <p>The implementation uses an array-based storage for entries and provides optimized iteration
+ * through a custom iterator. It includes memory management features such as automatic array
+ * reallocation when clearing large collections to prevent memory leaks.
+ *
+ * <p><strong>Important:</strong> The returned iterator from {@link #iterator()} must be consumed
+ * completely before calling {@code iterator()} again. The iterator maintains its position through a
+ * shared index, and calling {@code iterator()} again before the previous iterator is fully consumed
+ * will result in incorrect iteration behavior.
+ *
+ * <p>This class is marked as {@code @Internal} and should not be used directly by application code.
+ * It's specifically designed for internal serialization purposes.
+ *
+ * @param <E> the type of elements maintained by this list
+ * @since 1.0
+ */
+@Internal
+public class ListSnapshot<E> extends AbstractList<E> {
Review Comment:
Could we make CollectionSnapshot extends `AbstractList`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(java): Fix CopyOnWriteArrayList field serialization [fory]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #3079:
URL: https://github.com/apache/fory/pull/3079#discussion_r2643445941
##########
java/fory-core/src/main/java/org/apache/fory/collection/ListSnapshot.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fory.collection;
+
+import java.util.AbstractList;
+import java.util.Iterator;
+import java.util.List;
+import org.apache.fory.annotation.Internal;
+
+/**
+ * A specialized list implementation that creates a snapshot of elements for iteration with indexed
+ * access support. This class is designed to efficiently handle concurrent list serialization by
+ * creating a lightweight snapshot that can be iterated without holding references to the original
+ * list elements.
+ *
+ * <p>The implementation uses an array-based storage for entries and provides optimized iteration
+ * through a custom iterator. It includes memory management features such as automatic array
+ * reallocation when clearing large collections to prevent memory leaks.
+ *
+ * <p><strong>Important:</strong> The returned iterator from {@link #iterator()} must be consumed
+ * completely before calling {@code iterator()} again. The iterator maintains its position through a
+ * shared index, and calling {@code iterator()} again before the previous iterator is fully consumed
+ * will result in incorrect iteration behavior.
+ *
+ * <p>This class is marked as {@code @Internal} and should not be used directly by application code.
+ * It's specifically designed for internal serialization purposes.
+ *
+ * @param <E> the type of elements maintained by this list
+ * @since 1.0
+ */
+@Internal
+public class ListSnapshot<E> extends AbstractList<E> {
Review Comment:
Or just make CollectionSnapshot extends `AbstractList`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(java): Fix CopyOnWriteArrayList field serialization [fory]
Posted by "vybhavjs (via GitHub)" <gi...@apache.org>.
vybhavjs commented on code in PR #3079:
URL: https://github.com/apache/fory/pull/3079#discussion_r2643547538
##########
java/fory-core/src/main/java/org/apache/fory/collection/ListSnapshot.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fory.collection;
+
+import java.util.AbstractList;
+import java.util.Iterator;
+import java.util.List;
+import org.apache.fory.annotation.Internal;
+
+/**
+ * A specialized list implementation that creates a snapshot of elements for iteration with indexed
+ * access support. This class is designed to efficiently handle concurrent list serialization by
+ * creating a lightweight snapshot that can be iterated without holding references to the original
+ * list elements.
+ *
+ * <p>The implementation uses an array-based storage for entries and provides optimized iteration
+ * through a custom iterator. It includes memory management features such as automatic array
+ * reallocation when clearing large collections to prevent memory leaks.
+ *
+ * <p><strong>Important:</strong> The returned iterator from {@link #iterator()} must be consumed
+ * completely before calling {@code iterator()} again. The iterator maintains its position through a
+ * shared index, and calling {@code iterator()} again before the previous iterator is fully consumed
+ * will result in incorrect iteration behavior.
+ *
+ * <p>This class is marked as {@code @Internal} and should not be used directly by application code.
+ * It's specifically designed for internal serialization purposes.
+ *
+ * @param <E> the type of elements maintained by this list
+ * @since 1.0
+ */
+@Internal
+public class ListSnapshot<E> extends AbstractList<E> {
Review Comment:
That was my initial thought as most of the code is duplicated, and implementing `get(index)` is possible even with `CollectionSnapshot`. I decided against it because It seemed a bit odd semantically as now, non-list classes like `CopyOnWriteArraySet`, `ConcurrentSkipListSet` etc are being wrapped in a List implementation.
**All tests pass with that change though** - so should I go with that approach instead?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(java): Fix CopyOnWriteArrayList field serialization [fory]
Posted by "vybhavjs (via GitHub)" <gi...@apache.org>.
vybhavjs commented on code in PR #3079:
URL: https://github.com/apache/fory/pull/3079#discussion_r2643547538
##########
java/fory-core/src/main/java/org/apache/fory/collection/ListSnapshot.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fory.collection;
+
+import java.util.AbstractList;
+import java.util.Iterator;
+import java.util.List;
+import org.apache.fory.annotation.Internal;
+
+/**
+ * A specialized list implementation that creates a snapshot of elements for iteration with indexed
+ * access support. This class is designed to efficiently handle concurrent list serialization by
+ * creating a lightweight snapshot that can be iterated without holding references to the original
+ * list elements.
+ *
+ * <p>The implementation uses an array-based storage for entries and provides optimized iteration
+ * through a custom iterator. It includes memory management features such as automatic array
+ * reallocation when clearing large collections to prevent memory leaks.
+ *
+ * <p><strong>Important:</strong> The returned iterator from {@link #iterator()} must be consumed
+ * completely before calling {@code iterator()} again. The iterator maintains its position through a
+ * shared index, and calling {@code iterator()} again before the previous iterator is fully consumed
+ * will result in incorrect iteration behavior.
+ *
+ * <p>This class is marked as {@code @Internal} and should not be used directly by application code.
+ * It's specifically designed for internal serialization purposes.
+ *
+ * @param <E> the type of elements maintained by this list
+ * @since 1.0
+ */
+@Internal
+public class ListSnapshot<E> extends AbstractList<E> {
Review Comment:
That was my initial thought as most of the code is duplicated, and implementing `get(index)` is possible even with `CollectionSnapshot`. It seemed a bit odd semantically as now, non-list classes like `CopyOnWriteArraySet`, `ConcurrentSkipListSet` etc are being wrapped in a List implementation, and I wasn't sure if that would break something at runtime.
All tests pass with that approach though - so I've made the change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(java): Fix CopyOnWriteArrayList field serialization [fory]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang merged PR #3079:
URL: https://github.com/apache/fory/pull/3079
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(java): Fix CopyOnWriteArrayList field serialization [fory]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #3079:
URL: https://github.com/apache/fory/pull/3079#discussion_r2643443459
##########
java/fory-core/src/main/java/org/apache/fory/collection/ListSnapshot.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fory.collection;
+
+import java.util.AbstractList;
+import java.util.Iterator;
+import java.util.List;
+import org.apache.fory.annotation.Internal;
+
+/**
+ * A specialized list implementation that creates a snapshot of elements for iteration with indexed
+ * access support. This class is designed to efficiently handle concurrent list serialization by
+ * creating a lightweight snapshot that can be iterated without holding references to the original
+ * list elements.
+ *
+ * <p>The implementation uses an array-based storage for entries and provides optimized iteration
+ * through a custom iterator. It includes memory management features such as automatic array
+ * reallocation when clearing large collections to prevent memory leaks.
+ *
+ * <p><strong>Important:</strong> The returned iterator from {@link #iterator()} must be consumed
+ * completely before calling {@code iterator()} again. The iterator maintains its position through a
+ * shared index, and calling {@code iterator()} again before the previous iterator is fully consumed
+ * will result in incorrect iteration behavior.
+ *
+ * <p>This class is marked as {@code @Internal} and should not be used directly by application code.
+ * It's specifically designed for internal serialization purposes.
+ *
+ * @param <E> the type of elements maintained by this list
+ * @since 1.0
+ */
+@Internal
+public class ListSnapshot<E> extends AbstractList<E> {
Review Comment:
Could we remove CollectionSnapshot and use ListSnapshot acroos whol codebase?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(java): Fix CopyOnWriteArrayList field serialization [fory]
Posted by "vybhavjs (via GitHub)" <gi...@apache.org>.
vybhavjs commented on code in PR #3079:
URL: https://github.com/apache/fory/pull/3079#discussion_r2643547538
##########
java/fory-core/src/main/java/org/apache/fory/collection/ListSnapshot.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fory.collection;
+
+import java.util.AbstractList;
+import java.util.Iterator;
+import java.util.List;
+import org.apache.fory.annotation.Internal;
+
+/**
+ * A specialized list implementation that creates a snapshot of elements for iteration with indexed
+ * access support. This class is designed to efficiently handle concurrent list serialization by
+ * creating a lightweight snapshot that can be iterated without holding references to the original
+ * list elements.
+ *
+ * <p>The implementation uses an array-based storage for entries and provides optimized iteration
+ * through a custom iterator. It includes memory management features such as automatic array
+ * reallocation when clearing large collections to prevent memory leaks.
+ *
+ * <p><strong>Important:</strong> The returned iterator from {@link #iterator()} must be consumed
+ * completely before calling {@code iterator()} again. The iterator maintains its position through a
+ * shared index, and calling {@code iterator()} again before the previous iterator is fully consumed
+ * will result in incorrect iteration behavior.
+ *
+ * <p>This class is marked as {@code @Internal} and should not be used directly by application code.
+ * It's specifically designed for internal serialization purposes.
+ *
+ * @param <E> the type of elements maintained by this list
+ * @since 1.0
+ */
+@Internal
+public class ListSnapshot<E> extends AbstractList<E> {
Review Comment:
That was my initial thought as most of the code is duplicated, and implementing `get(index)` is possible even with `CollectionSnapshot`. t seemed a bit odd semantically as now, non-list classes like `CopyOnWriteArraySet`, `ConcurrentSkipListSet` etc are being wrapped in a List implementation, and I wasn't sure if that would break something at runtime.
**All tests pass with that change though** - so should I go with that approach instead?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org