From 0a1f9d8ff491ef5b38e953faede838886a3d3426 Mon Sep 17 00:00:00 2001 From: "elena.parovyshnaya" Date: Sun, 12 Jul 2020 08:10:01 +0300 Subject: [PATCH] Bug 565012 - API revision | permission | OshiPermissionEmitter - Redesign hardware state reading synchronization in order to `lock` only reading code. Mutability of data structures in `State` are also eliminated, new instance of `State` is created for each request. - Test the service, including massive simultaneous requests case. Signed-off-by: elena.parovyshnaya --- .../oshi/tobemoved/HardwareEnvironment.java | 24 ++- .../lic/internal/oshi/tobemoved/State.java | 59 +++++--- .../tobemoved/HardwareEnvironmentTest.java | 142 ++++++++++++++++++ 3 files changed, 202 insertions(+), 23 deletions(-) create mode 100644 tests/org.eclipse.passage.lic.oshi.tests/src/org/eclipse/passage/lic/oshi/tests/tobemoved/HardwareEnvironmentTest.java diff --git a/bundles/org.eclipse.passage.lic.oshi/src/org/eclipse/passage/lic/internal/oshi/tobemoved/HardwareEnvironment.java b/bundles/org.eclipse.passage.lic.oshi/src/org/eclipse/passage/lic/internal/oshi/tobemoved/HardwareEnvironment.java index 8d64aa9e4..349a9f447 100644 --- a/bundles/org.eclipse.passage.lic.oshi/src/org/eclipse/passage/lic/internal/oshi/tobemoved/HardwareEnvironment.java +++ b/bundles/org.eclipse.passage.lic.oshi/src/org/eclipse/passage/lic/internal/oshi/tobemoved/HardwareEnvironment.java @@ -17,11 +17,21 @@ import org.eclipse.passage.lic.internal.api.inspection.EnvironmentProperty; import org.eclipse.passage.lic.internal.api.inspection.RuntimeEnvironment; +import oshi.SystemInfo; + +/** + *

+ * It's extremely important to have not more then one instance of this service + * at runtime (guaranteed by {@code Framework}). + *

+ * + * @see State + */ + @SuppressWarnings("restriction") public final class HardwareEnvironment implements RuntimeEnvironment { private final EvaluationType type = new EvaluationType.Hardware(); - private final State state = new State(); @Override public EvaluationType id() { @@ -30,12 +40,20 @@ public EvaluationType id() { @Override public boolean isAssuptionTrue(EnvironmentProperty property, String assumption) throws LicensingException { - return state.hasValue(property, assumption); + return freshState().hasValue(property, assumption); } @Override public String state() throws LicensingException { - return new GlanceOfState(state).get(); + return new GlanceOfState(freshState()).get(); + } + + private State freshState() throws LicensingException { + State state; + synchronized (SystemInfo.class) { + state = new State(); + } + return state; } } diff --git a/bundles/org.eclipse.passage.lic.oshi/src/org/eclipse/passage/lic/internal/oshi/tobemoved/State.java b/bundles/org.eclipse.passage.lic.oshi/src/org/eclipse/passage/lic/internal/oshi/tobemoved/State.java index 99c423682..9145cfeeb 100644 --- a/bundles/org.eclipse.passage.lic.oshi/src/org/eclipse/passage/lic/internal/oshi/tobemoved/State.java +++ b/bundles/org.eclipse.passage.lic.oshi/src/org/eclipse/passage/lic/internal/oshi/tobemoved/State.java @@ -38,33 +38,56 @@ import oshi.hardware.HardwareAbstractionLayer; import oshi.software.os.OperatingSystem; +/** + *

+ * State has aggressively not thread-safe hardware state reading phase, which is + * done on construction time. + *

+ *

+ * Reading hardware (done eagerly on a State construction) boils down to lots + * tricky native code invocation, which has static parts and does not stand + * concurrent access. + *

+ *

+ * Thus, State construction must be synchronized externally: until one State if + * fully constructed, no another one must even think about it. + *

+ *

+ * We construct it under a class-dedicated lock only in a + * {@linkplain HardwareEnvironment} service, which, it turn, must have only + * single instance at runtime (which is guaranteed by {@code Framework}). + *

+ *

+ * Regarding to the data it collects - it's fully immutable, always fresh and + * absolutely thread safe. + *

+ */ @SuppressWarnings("restriction") final class State { private final Map hardware = new HashMap<>(); private final String diskFamily = new Disk.Name().family(); private final List> disks = new ArrayList<>(); - private final Object lock = new Object(); - boolean hasValue(EnvironmentProperty property, String expected) throws LicensingException { + State() throws LicensingException { + read(); + } + + boolean hasValue(EnvironmentProperty property, String expected) { String regexp = expected.replaceAll("\\*", ".*"); //$NON-NLS-1$//$NON-NLS-2$ - synchronized (lock) { - read(); - if (diskFamily.equals(property.family())) { - return disks.stream()// - .map(properties -> Optional.ofNullable(properties.get(property)))// - .filter(Optional::isPresent)// - .map(Optional::get)// - .anyMatch(value -> value.matches(regexp)); - } - return Optional.ofNullable(hardware.get(property))// - .map(value -> value.matches(regexp))// - .orElse(false); + if (diskFamily.equals(property.family())) { + return disks.stream()// + .map(properties -> Optional.ofNullable(properties.get(property)))// + .filter(Optional::isPresent)// + .map(Optional::get)// + .anyMatch(value -> value.matches(regexp)); } + return Optional.ofNullable(hardware.get(property))// + .map(value -> value.matches(regexp))// + .orElse(false); } private void read() throws LicensingException { - clean(); try { SystemInfo system = new SystemInfo(); readOS(system.getOperatingSystem()); @@ -74,11 +97,6 @@ private void read() throws LicensingException { } } - private void clean() { - disks.clear(); - hardware.clear(); - } - private void readOS(OperatingSystem info) { store(info::getFamily, new OS.Family(), hardware); store(info::getManufacturer, new OS.Manufacturer(), hardware); @@ -140,4 +158,5 @@ private Map diskProperties(HWDiskStore info) { private void store(Supplier value, EnvironmentProperty key, Map target) { Optional.ofNullable(value.get()).ifPresent(valuable -> target.put(key, valuable)); } + } diff --git a/tests/org.eclipse.passage.lic.oshi.tests/src/org/eclipse/passage/lic/oshi/tests/tobemoved/HardwareEnvironmentTest.java b/tests/org.eclipse.passage.lic.oshi.tests/src/org/eclipse/passage/lic/oshi/tests/tobemoved/HardwareEnvironmentTest.java new file mode 100644 index 000000000..5f04a9ab2 --- /dev/null +++ b/tests/org.eclipse.passage.lic.oshi.tests/src/org/eclipse/passage/lic/oshi/tests/tobemoved/HardwareEnvironmentTest.java @@ -0,0 +1,142 @@ +/******************************************************************************* + * Copyright (c) 2020 ArSysOp + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0/. + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * ArSysOp - initial API and implementation + *******************************************************************************/ +package org.eclipse.passage.lic.oshi.tests.tobemoved; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executor; +import java.util.concurrent.Executors; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +import org.eclipse.passage.lic.internal.api.LicensingException; +import org.eclipse.passage.lic.internal.api.conditions.EvaluationType; +import org.eclipse.passage.lic.internal.base.inspection.hardware.Disk; +import org.eclipse.passage.lic.internal.base.inspection.hardware.OS; +import org.eclipse.passage.lic.internal.oshi.tobemoved.HardwareEnvironment; +import org.junit.Test; + +@SuppressWarnings("restriction") +public final class HardwareEnvironmentTest { + + @Test + public void isDedicatedToHardware() { + assertEquals(new EvaluationType.Hardware(), new HardwareEnvironment().id()); + } + + @Test + public void inspects() { + try { + assertFalse(new HardwareEnvironment().isAssuptionTrue(// + new OS.Family(), // + "not-existing-operating-system")); //$NON-NLS-1$ + } catch (LicensingException e) { + fail("Is not supposed to fail on valid data"); //$NON-NLS-1$ + } + } + + @Test(expected = NullPointerException.class) + public void doesNotInspectNullProperty() { + try { + new HardwareEnvironment().isAssuptionTrue(null, "none"); //$NON-NLS-1$ + } catch (LicensingException e) { + fail("No insection activity is intended to be triggered for invalid input data"); //$NON-NLS-1$ + } + } + + @Test(expected = NullPointerException.class) + public void doesNotInspectForNullValue() { + try { + new HardwareEnvironment().isAssuptionTrue(new OS.Family(), null); // $NON-NLS-1$ + } catch (LicensingException e) { + fail("No insection activity is intended to be triggered for invalid input data"); //$NON-NLS-1$ + } + } + + @Test + public void knowsSimpleRegexp() { + try { + assertTrue(new HardwareEnvironment().isAssuptionTrue(new OS.Family(), "*"));//$NON-NLS-1$ + } catch (LicensingException e) { + fail("Is not supposed to fail on valid data"); //$NON-NLS-1$ + } + } + + @Test + public void standsSimultaneousRequests() { + // given: single instance of the env and lots of requesters + HardwareEnvironment hardware = new HardwareEnvironment(); + int threads = 128; + CountDownLatch readySteadyGo = new CountDownLatch(1); + CountDownLatch done = new CountDownLatch(threads); + List demands = IntStream.range(0, threads) // + .mapToObj(no -> new InspectionDemand(hardware, readySteadyGo, done, no))// + .collect(Collectors.toList()); + + // when: run all of the requesters simultaneously (latched by readySteadyGo) + Executor executor = Executors.newFixedThreadPool(threads); + demands.stream().forEach(executor::execute); + try { + Thread.sleep(2000); // let'em all start and hold waiting for each other + readySteadyGo.countDown(); // and now trigger'em all to ddos the env + done.await(); // and just wait until each of'em finish + } catch (InterruptedException e) { + fail("Test has been interrupted"); //$NON-NLS-1$ + } + + // then: all of'em succeed + assertTrue(demands.stream().allMatch(InspectionDemand::result)); + } + + private static final class InspectionDemand implements Runnable { + private final CountDownLatch altogether; + private final CountDownLatch done; + private final HardwareEnvironment env; + private boolean result = false; + + InspectionDemand(HardwareEnvironment env, CountDownLatch altogether, CountDownLatch done, int no) { + this.env = env; + this.altogether = altogether; + this.done = done; + } + + @Override + public void run() { + try { + altogether.await(); // wait for a common signal to start simultaneously + ask(); + done.countDown(); // report you are done + } catch (InterruptedException ex) { + ex.printStackTrace(); + } + } + + private void ask() { + try { + result = !env.isAssuptionTrue(new Disk.Serial(), "not-existing-disk-serial"); //$NON-NLS-1$ + } catch (Exception e) { + result = false; + } + } + + boolean result() { + return result; + } + } + +}