This is an automated email from the ASF dual-hosted git repository. chrisdutz pushed a commit to branch develop in repository https://gitbox.apache.org/repos/asf/plc4x.git
commit 8d341dafada8d0d113040d7b612e02141756f6b5 Author: Christofer Dutz <[email protected]> AuthorDate: Fri Aug 14 11:35:58 2026 +0200 fix(plc4j/opm): Optimized the PlcEntityManager to reuse the proxies it creates to avoid issues with people not using the API correctly. fixes: #1935 --- .../apache/plc4x/java/opm/PlcEntityManager.java | 55 +++++++-- .../opm/issues/i1935/ProxyClassCachingTest.java | 137 +++++++++++++++++++++ 2 files changed, 179 insertions(+), 13 deletions(-) diff --git a/plc4j/tools/opm/src/main/java/org/apache/plc4x/java/opm/PlcEntityManager.java b/plc4j/tools/opm/src/main/java/org/apache/plc4x/java/opm/PlcEntityManager.java index e522e93d13..3f820c9dd2 100644 --- a/plc4j/tools/opm/src/main/java/org/apache/plc4x/java/opm/PlcEntityManager.java +++ b/plc4j/tools/opm/src/main/java/org/apache/plc4x/java/opm/PlcEntityManager.java @@ -19,6 +19,7 @@ package org.apache.plc4x.java.opm; import net.bytebuddy.ByteBuddy; +import net.bytebuddy.TypeCache; import net.bytebuddy.description.modifier.Visibility; import net.bytebuddy.implementation.MethodDelegation; import org.apache.commons.lang3.reflect.FieldUtils; @@ -86,6 +87,13 @@ public class PlcEntityManager { public static final String LAST_FETCHED = "_lastFetched"; public static final String LAST_WRITTEN = "_lastWritten"; + /** + * Generated proxy classes, keyed by class loader and entity class. Soft references so the + * cache can't keep class loaders alive on its own. + */ + private static final TypeCache<Class<?>> PROXY_CACHE = + new TypeCache.WithInlineExpunction<>(TypeCache.Sort.SOFT); + private final PlcConnectionManager connectionManager; private final SimpleAliasRegistry registry; @@ -142,19 +150,7 @@ public class PlcEntityManager { private <T> T connect(Class<T> clazz, String address, T existingInstance) throws OPMException { OpmUtils.getPlcEntityAndCheckPreconditions(clazz); try { - // Use Byte Buddy to generate a subclassed proxy that delegates all PlcField Methods - // to the intercept method - T instance = new ByteBuddy() - .subclass(clazz) - .defineField(PLC_ADDRESS_FIELD_NAME, String.class, Visibility.PRIVATE) - .defineField(CONNECTION_MANAGER_FIELD_NAME, PlcConnectionManager.class, Visibility.PRIVATE) - .defineField(ALIAS_REGISTRY, AliasRegistry.class, Visibility.PRIVATE) - .defineField(LAST_FETCHED, Map.class, Visibility.PRIVATE) - .defineField(LAST_WRITTEN, Map.class, Visibility.PRIVATE) - .method(not(isDeclaredBy(Object.class))).intercept(MethodDelegation.to(PlcEntityInterceptor.class)) - .make() - .load(Thread.currentThread().getContextClassLoader()) - .getLoaded() + T instance = proxyClassFor(clazz) .getConstructor() .newInstance(); // Set connection value into the private field @@ -184,6 +180,39 @@ public class PlcEntityManager { } } + /** + * Returns the generated proxy class for the given entity class, generating it on first use. + * <p> + * The generated type depends only on the entity class, so it is cached and reused - see + * GH-1935. Generating and loading a fresh subclass on every {@link #connect(Class, String)} + * (and therefore on every {@link #read(Class, String)} / {@link #write(Class, String, Object)}) + * leaks one class per call, which fills up metaspace in any polling application. + */ + @SuppressWarnings("unchecked") + private <T> Class<? extends T> proxyClassFor(Class<T> clazz) throws OPMException { + ClassLoader classLoader = Thread.currentThread().getContextClassLoader(); + try { + // The cache is keyed by class loader and entity class, so the same entity loaded by + // two class loaders correctly gets a proxy per loader. The monitor makes concurrent + // callers wait for one generation instead of racing to generate the same class. + return (Class<? extends T>) PROXY_CACHE.findOrInsert(classLoader, clazz, + () -> new ByteBuddy() + .subclass(clazz) + .defineField(PLC_ADDRESS_FIELD_NAME, String.class, Visibility.PRIVATE) + .defineField(CONNECTION_MANAGER_FIELD_NAME, PlcConnectionManager.class, Visibility.PRIVATE) + .defineField(ALIAS_REGISTRY, AliasRegistry.class, Visibility.PRIVATE) + .defineField(LAST_FETCHED, Map.class, Visibility.PRIVATE) + .defineField(LAST_WRITTEN, Map.class, Visibility.PRIVATE) + .method(not(isDeclaredBy(Object.class))).intercept(MethodDelegation.to(PlcEntityInterceptor.class)) + .make() + .load(classLoader) + .getLoaded(), + PROXY_CACHE); + } catch (RuntimeException e) { + throw new OPMException("Unable to generate Proxy for " + clazz.getName(), e); + } + } + private Object getValueFromField(Field field, Object object) { try { return field.get(object); diff --git a/plc4j/tools/opm/src/test/java/org/apache/plc4x/java/opm/issues/i1935/ProxyClassCachingTest.java b/plc4j/tools/opm/src/test/java/org/apache/plc4x/java/opm/issues/i1935/ProxyClassCachingTest.java new file mode 100644 index 0000000000..cf606df7d4 --- /dev/null +++ b/plc4j/tools/opm/src/test/java/org/apache/plc4x/java/opm/issues/i1935/ProxyClassCachingTest.java @@ -0,0 +1,137 @@ +/* + * 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 + * + * https://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.plc4x.java.opm.issues.i1935; + +import org.apache.plc4x.java.DefaultPlcDriverManager; +import org.apache.plc4x.java.api.types.PlcResponseCode; +import org.apache.plc4x.java.mock.connection.MockConnection; +import org.apache.plc4x.java.mock.connection.MockDevice; +import org.apache.plc4x.java.opm.OPMException; +import org.apache.plc4x.java.opm.PlcEntity; +import org.apache.plc4x.java.opm.PlcEntityManager; +import org.apache.plc4x.java.opm.PlcTag; +import org.apache.plc4x.java.spi.drivers.messages.items.DefaultPlcResponseItem; +import org.apache.plc4x.java.spi.values.PlcSTRING; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.HashSet; +import java.util.Set; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +/** + * The generated proxy type depends only on the entity class, so it has to be generated once and + * reused - see GH-1935. Generating one per call leaks a class per read, which is what filled up + * the reporter's heap. + */ +@ExtendWith(MockitoExtension.class) +public class ProxyClassCachingTest { + + private PlcEntityManager entityManager; + + @Mock + MockDevice mockDevice; + + @BeforeEach + void setUp() throws Exception { + DefaultPlcDriverManager driverManager = new DefaultPlcDriverManager(); + MockConnection connection = (MockConnection) driverManager.getConnection("mock:test"); + when(mockDevice.read(any())) + .thenReturn(new DefaultPlcResponseItem<>(PlcResponseCode.OK, new PlcSTRING("hallo"))); + connection.setDevice(mockDevice); + entityManager = new PlcEntityManager(driverManager); + } + + @Test + void repeatedReadsReuseTheSameProxyClass() throws OPMException { + Set<Class<?>> proxyClasses = new HashSet<>(); + for (int i = 0; i < 25; i++) { + proxyClasses.add(entityManager.read(SimpleEntity.class, "mock:test").getClass()); + } + + assertEquals(1, proxyClasses.size(), + "every read generated a new proxy class - the type cache is not being used"); + } + + @Test + void connectAndReadShareTheSameProxyClass() throws OPMException { + Object connected = entityManager.connect(SimpleEntity.class, "mock:test"); + Object read = entityManager.read(SimpleEntity.class, "mock:test"); + + assertSame(connected.getClass(), read.getClass()); + } + + @Test + void separateEntityManagersShareTheSameProxyClass() throws Exception { + DefaultPlcDriverManager otherDriverManager = new DefaultPlcDriverManager(); + MockConnection otherConnection = (MockConnection) otherDriverManager.getConnection("mock:other"); + otherConnection.setDevice(mockDevice); + PlcEntityManager otherEntityManager = new PlcEntityManager(otherDriverManager); + + Object first = entityManager.read(SimpleEntity.class, "mock:test"); + Object second = otherEntityManager.read(SimpleEntity.class, "mock:other"); + + assertSame(first.getClass(), second.getClass()); + } + + @Test + void differentEntitiesGetDifferentProxyClasses() throws OPMException { + Object simple = entityManager.read(SimpleEntity.class, "mock:test"); + Object other = entityManager.read(OtherEntity.class, "mock:test"); + + assertEquals(2, new HashSet<>(Set.of(simple.getClass(), other.getClass())).size()); + } + + @PlcEntity + public static class SimpleEntity { + + @PlcTag("address") + private String field; + + public SimpleEntity() { + // For OPM + } + + public String getField() { + return field; + } + } + + @PlcEntity + public static class OtherEntity { + + @PlcTag("address") + private String field; + + public OtherEntity() { + // For OPM + } + + public String getField() { + return field; + } + } +}
