diff --git a/docs/about/changelog.md b/docs/about/changelog.md index 562c41ce4f..6d14c47331 100644 --- a/docs/about/changelog.md +++ b/docs/about/changelog.md @@ -6,6 +6,9 @@ priorities and future plans. ## Work in progress +- Snapshot mutable array entries while marking package roots and during reachability walks so concurrent thread teardown cannot fail with `ConcurrentModificationException`. + +- Enforce restricted-hash key access and implement `Hash::Util` key, value, and hash locking (#1502, #1172). - Report the caller's scalar context correctly when inspecting frames in overloaded exceptions (#1859). - Avoid heap exhaustion when compiling very large generated subroutines, and preserve heredoc statement boundaries with `\&name` fallbacks (#1252). - Cover nested require caller packages and imported `CLASS` constants, and match defer ordering and caller frames around loop `continue` blocks (#1601, #1405, #1383). diff --git a/docs/reference/bundled-modules.md b/docs/reference/bundled-modules.md index dadff9bc61..c9d4f70dce 100644 --- a/docs/reference/bundled-modules.md +++ b/docs/reference/bundled-modules.md @@ -242,7 +242,7 @@ These are loaded automatically or via `use`: | `Scalar::Readonly` | Java + Perl | Scalar read-only query, enable, and disable operations replacing the CPAN XS implementation | | `Scalar::Type` | Java + Perl | Scalar storage-type introspection without native XS | | `List::Util` | Java | `reduce`, `first`, `min`, `max`, `sum`, `mesh`/`zip`, etc. | -| `Hash::Util` | Java | `lock_keys`, `lock_hash`, etc. | +| `Hash::Util` | Java | Restricted-key enforcement and key/value/hash locks; `lock_keys`, `lock_keys_plus`, inspection helpers, and `lock_hash` are implemented. | ### File & I/O diff --git a/docs/reference/feature-matrix.md b/docs/reference/feature-matrix.md index df68047e58..46f0b27a7f 100644 --- a/docs/reference/feature-matrix.md +++ b/docs/reference/feature-matrix.md @@ -376,8 +376,8 @@ my @copy = @{$z}; # ERROR - ✅ **Array delete**: `delete` for array indexes is implemented. - ✅ **Tied Arrays**: Tied arrays are implemented. See also [Tied Scalars](#scalars), [Tied Hashes](#arrays-hashes-and-lists), [Tied Handles](#io-operations). - ✅ **Tied Hashes**: Tied hashes are implemented. See also [Tied Scalars](#scalars), [Tied Arrays](#arrays-hashes-and-lists), [Tied Handles](#io-operations). -- 🟡 **Restricted hashes**: `Hash::Util` supplies compatibility entry points, - but lock/unlock helpers do not enforce native Perl hash restrictions. +- ✅ **Restricted hashes**: `Hash::Util` enforces allowed-key access and + supports key/value/hash locking, unlocking, and restriction inspection. --- diff --git a/src/main/java/org/perlonjava/frontend/parser/PrototypeArgs.java b/src/main/java/org/perlonjava/frontend/parser/PrototypeArgs.java index 408887f9e5..0fc9156b99 100644 --- a/src/main/java/org/perlonjava/frontend/parser/PrototypeArgs.java +++ b/src/main/java/org/perlonjava/frontend/parser/PrototypeArgs.java @@ -1008,6 +1008,21 @@ private static void handleListOrHashArgument(Parser parser, ListNode args, boole } ListNode argList = ListParser.parseZeroOrMoreList(parser, 0, false, false, false, false); + if ("all_keys".equals(operatorName) || (operatorName != null && operatorName.endsWith("::all_keys"))) { + // Hash::Util::all_keys has two output arrays after its hash + // argument. Preserve those lvalues as references for its XS-like + // Java implementation instead of flattening their current values. + for (Node node : argList.elements) { + if (node instanceof OperatorNode sigil && sigil.operator.equals("@")) { + Node reference = new OperatorNode("\\", node, node.getIndex()); + reference.setAnnotation("context", "SCALAR"); + args.elements.add(reference); + } else { + args.elements.add(node); + } + } + return; + } // @ and % consume remaining arguments in LIST context // for (Node element : argList.elements) { // element.setAnnotation("context", "LIST"); diff --git a/src/main/java/org/perlonjava/runtime/perlmodule/HashUtil.java b/src/main/java/org/perlonjava/runtime/perlmodule/HashUtil.java index b98bc47b74..73b844fab1 100644 --- a/src/main/java/org/perlonjava/runtime/perlmodule/HashUtil.java +++ b/src/main/java/org/perlonjava/runtime/perlmodule/HashUtil.java @@ -36,6 +36,16 @@ public static void initialize() { hashUtil.registerMethod("unlock_keys", "unlock_keys", "\\%@"); hashUtil.registerMethod("lock_hash", "lock_hash", "\\%"); hashUtil.registerMethod("unlock_hash", "unlock_hash", "\\%"); + hashUtil.registerMethod("lock_value", "lock_value", "\\%$"); + hashUtil.registerMethod("unlock_value", "unlock_value", "\\%$"); + hashUtil.registerMethod("lock_keys_plus", "lock_keys_plus", "\\%;@"); + hashUtil.registerMethod("hashref_locked", "hashref_locked", null); + hashUtil.registerMethod("hash_locked", "hash_locked", "\\%"); + hashUtil.registerMethod("hash_unlocked", "hash_unlocked", "\\%"); + hashUtil.registerMethod("legal_keys", "legal_keys", "\\%"); + hashUtil.registerMethod("all_keys", "all_keys", "\\%@@"); + hashUtil.registerMethod("hidden_keys", "hidden_keys", "\\%"); + hashUtil.registerMethod("legal_ref_keys", "legal_ref_keys", "$"); hashUtil.registerMethod("hash_seed", "hash_seed", null); hashUtil.registerMethod("num_buckets", "num_buckets", "\\%"); @@ -185,28 +195,82 @@ private static int estimateUsedBuckets(Map map, int capac // Placeholder implementations for other Hash::Util functions public static RuntimeList lock_keys(RuntimeArray args, int ctx) { - // TODO: Implement hash key locking - if (args.size() > 0) { - return args.get(0).getList(); // Return the hash reference - } - return RuntimeScalarCache.scalarUndef.getList(); + if (args.size() == 0) return RuntimeScalarCache.scalarUndef.getList(); + RuntimeHash hash = hashArgument(args.get(0)); + lockKeys(hash, args, 1); + return args.get(0).getList(); } public static RuntimeList unlock_keys(RuntimeArray args, int ctx) { - // TODO: Implement hash key unlocking - if (args.size() > 0) { - return args.get(0).getList(); // Return the hash reference + if (args.size() == 0) return RuntimeScalarCache.scalarUndef.getList(); + hashArgument(args.get(0)).legalHashKeys = null; + return args.get(0).getList(); + } + + private static void lockKeys(RuntimeHash hash, RuntimeArray args, int firstKey) { + java.util.LinkedHashSet allowed = new java.util.LinkedHashSet<>(); + if (args.size() == firstKey) { + allowed.addAll(hash.elements.keySet()); + } else { + for (int i = firstKey; i < args.size(); i++) allowed.add(args.get(i).toString()); + for (String existing : hash.elements.keySet()) { + if (!allowed.contains(existing)) { + throw new PerlCompilerException("Hash has key '" + existing + "' which is not in the new key set"); + } + } + } + hash.legalHashKeys = allowed; + } + + public static RuntimeList lock_keys_plus(RuntimeArray args, int ctx) { + if (args.size() == 0) return RuntimeScalarCache.scalarUndef.getList(); + RuntimeHash hash = hashArgument(args.get(0)); + java.util.LinkedHashSet allowed = new java.util.LinkedHashSet<>(hash.elements.keySet()); + for (int i = 1; i < args.size(); i++) allowed.add(args.get(i).toString()); + hash.legalHashKeys = allowed; + return args.get(0).getList(); + } + + public static RuntimeList lock_value(RuntimeArray args, int ctx) { + if (args.size() < 2) return RuntimeScalarCache.scalarUndef.getList(); + RuntimeHash hash = hashArgument(args.get(0)); + if (!hash.isHashRestricted()) { + throw new PerlCompilerException("Cannot usefully lock values in an unlocked hash"); + } + String key = args.get(1).toString(); + hash.validateHashKey(key, "read"); + hash.lockedHashValues = hash.lockedHashValues == null ? new java.util.HashSet<>() : hash.lockedHashValues; + hash.lockedHashValues.add(key); + RuntimeScalar value = hash.elements.get(key); + if (value != null) makeScalarReadOnly(value); + return hash.get(key).getList(); + } + + public static RuntimeList unlock_value(RuntimeArray args, int ctx) { + if (args.size() < 2) return RuntimeScalarCache.scalarUndef.getList(); + RuntimeHash hash = hashArgument(args.get(0)); + String key = args.get(1).toString(); + if (hash.lockedHashValues != null) hash.lockedHashValues.remove(key); + RuntimeScalar value = hash.elements.get(key); + if (value != null) makeScalarWritable(value); + return hash.get(key).getList(); + } + + private static void makeScalarWritable(RuntimeScalar scalar) { + if (scalar.type == RuntimeScalarType.READONLY_SCALAR && scalar.value instanceof RuntimeScalar inner) { + scalar.type = inner.type; + scalar.value = inner.value; } - return RuntimeScalarCache.scalarUndef.getList(); } public static RuntimeList lock_hash(RuntimeArray args, int ctx) { if (args.size() > 0) { RuntimeHash hash = hashArgument(args.get(0)); + hash.legalHashKeys = new java.util.LinkedHashSet<>(hash.elements.keySet()); + hash.allHashValuesLocked = true; for (RuntimeScalar scalar : hash.elements.values()) { makeScalarReadOnly(scalar); } - hash.type = RuntimeHash.READONLY_HASH; return args.get(0).getList(); // Return the hash reference } return RuntimeScalarCache.scalarUndef.getList(); @@ -215,6 +279,9 @@ public static RuntimeList lock_hash(RuntimeArray args, int ctx) { public static RuntimeList unlock_hash(RuntimeArray args, int ctx) { if (args.size() > 0) { RuntimeHash hash = hashArgument(args.get(0)); + hash.legalHashKeys = null; + hash.lockedHashValues = null; + hash.allHashValuesLocked = false; for (RuntimeScalar scalar : hash.elements.values()) { if (scalar.type == RuntimeScalarType.READONLY_SCALAR && scalar.value instanceof RuntimeScalar inner) { @@ -222,7 +289,6 @@ public static RuntimeList unlock_hash(RuntimeArray args, int ctx) { scalar.value = inner.value; } } - hash.type = RuntimeHash.PLAIN_HASH; return args.get(0).getList(); // Return the hash reference } return RuntimeScalarCache.scalarUndef.getList(); @@ -235,6 +301,69 @@ private static RuntimeHash hashArgument(RuntimeScalar arg) { return arg.hashDeref(); } + public static RuntimeList hashref_locked(RuntimeArray args, int ctx) { + return new RuntimeScalar(args.size() > 0 && hashArgument(args.get(0)).isHashRestricted()).getList(); + } + + public static RuntimeList hash_locked(RuntimeArray args, int ctx) { + if (args.size() == 0) return new RuntimeScalar(false).getList(); + RuntimeHash hash = hashArgument(args.get(0)); + return new RuntimeScalar(hash.isHashRestricted() || hash.allHashValuesLocked + || (hash.lockedHashValues != null && !hash.lockedHashValues.isEmpty())).getList(); + } + + public static RuntimeList hash_unlocked(RuntimeArray args, int ctx) { + if (args.size() == 0) return new RuntimeScalar(true).getList(); + RuntimeHash hash = hashArgument(args.get(0)); + boolean locked = hash.isHashRestricted() || hash.allHashValuesLocked + || (hash.lockedHashValues != null && !hash.lockedHashValues.isEmpty()); + return new RuntimeScalar(!locked).getList(); + } + + public static RuntimeList legal_keys(RuntimeArray args, int ctx) { return keyList(args, true); } + public static RuntimeList all_keys(RuntimeArray args, int ctx) { + if (args.size() == 0) return new RuntimeList(); + RuntimeHash hash = hashArgument(args.get(0)); + RuntimeArray visible = outputArray(args, 1); + RuntimeArray hidden = outputArray(args, 2); + if (visible != null) visible.elements.clear(); + if (hidden != null) hidden.elements.clear(); + for (String key : hash.elements.keySet()) { + if (visible != null) visible.elements.add(new RuntimeScalar(key)); + } + if (hash.legalHashKeys != null) for (String key : hash.legalHashKeys) { + if (!hash.elements.containsKey(key) && hidden != null) hidden.elements.add(new RuntimeScalar(key)); + } + return args.get(0).getList(); + } + public static RuntimeList hidden_keys(RuntimeArray args, int ctx) { + if (args.size() == 0) return new RuntimeList(); + RuntimeHash hash = hashArgument(args.get(0)); + RuntimeArray result = new RuntimeArray(); + if (hash.legalHashKeys != null) for (String key : hash.legalHashKeys) { + if (!hash.elements.containsKey(key)) result.elements.add(new RuntimeScalar(key)); + } + return result.getList(); + } + public static RuntimeList legal_ref_keys(RuntimeArray args, int ctx) { return keyList(args, true); } + private static RuntimeArray outputArray(RuntimeArray args, int index) { + if (index >= args.size()) return null; + RuntimeScalar scalar = args.get(index); + if (scalar.type == RuntimeScalarType.ARRAYREFERENCE && scalar.value instanceof RuntimeArray array) return array; + return null; + } + private static RuntimeList keyList(RuntimeArray args, boolean legalOnly) { + if (args.size() == 0) return new RuntimeList(); + RuntimeHash hash = hashArgument(args.get(0)); + RuntimeArray result = new RuntimeArray(); + if (legalOnly && hash.legalHashKeys != null) { + for (String key : hash.legalHashKeys) result.elements.add(new RuntimeScalar(key)); + } else { + for (String key : hash.elements.keySet()) result.elements.add(new RuntimeScalar(key)); + } + return result.getList(); + } + private static void makeScalarReadOnly(RuntimeScalar scalar) { if (scalar.type == RuntimeScalarType.READONLY_SCALAR || scalar instanceof RuntimeScalarReadOnly) { diff --git a/src/main/java/org/perlonjava/runtime/perlmodule/Internals.java b/src/main/java/org/perlonjava/runtime/perlmodule/Internals.java index cb82e5bd6c..c3a1d3afd4 100644 --- a/src/main/java/org/perlonjava/runtime/perlmodule/Internals.java +++ b/src/main/java/org/perlonjava/runtime/perlmodule/Internals.java @@ -32,11 +32,9 @@ public static void initialize() { try { internals.registerMethod("SvREADONLY", "svReadonly", "\\[$@%];$"); internals.registerMethod("SvREFCNT", "svRefcount", "$;$"); - // Clear placeholder slots in a restricted hash. PerlOnJava doesn't - // implement Perl's restricted-hash placeholder machinery (used by - // Hash::Util / fields), so this is a safe no-op. Modules like - // Const::Fast call it when sealing a hash readonly; failing here - // breaks loading the whole module. + // Clear placeholder slots in a restricted hash. PerlOnJava tracks + // allowed-but-absent keys separately, so there are no physical + // placeholder entries to remove. internals.registerMethod("hv_clear_placeholders", "hvClearPlaceholders", "\\%"); // Phase 0 diagnostic: expose PerlOnJava-internal refcount state // (refCount, flags, tracking mode) for differential testing @@ -980,7 +978,7 @@ else if (scalar.type != RuntimeScalarType.READONLY_SCALAR // Check hash reference if (effectiveScalar.type == RuntimeScalarType.HASHREFERENCE && effectiveScalar.value instanceof RuntimeHash hash) { - return new RuntimeScalar(hash.type == RuntimeHash.READONLY_HASH).getList(); + return new RuntimeScalar(hash.type == RuntimeHash.READONLY_HASH || hash.isHashRestricted()).getList(); } // Check scalar reference: SvREADONLY checks whether SvRV(arg) has readonly flag if (effectiveScalar.type == RuntimeScalarType.REFERENCE diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/GlobalVariable.java b/src/main/java/org/perlonjava/runtime/runtimetypes/GlobalVariable.java index 6995fb20c1..7edd678928 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/GlobalVariable.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/GlobalVariable.java @@ -750,7 +750,11 @@ static T markPackageGlobalRoot(T root) { hash.markPackageRootedValue(value); } } else if (root instanceof RuntimeArray array) { - for (RuntimeScalar value : array.elements) { + // Package-root marking may race with another thread mutating a + // shared array. This is bookkeeping, so mark the values from a + // best-effort snapshot instead of leaking ArrayList's + // fail-fast ConcurrentModificationException into Perl code. + for (RuntimeScalar value : array.elements.toArray(RuntimeScalar[]::new)) { array.markPackageRootedValue(value); } } diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/ReachabilityWalker.java b/src/main/java/org/perlonjava/runtime/runtimetypes/ReachabilityWalker.java index 37e74459ac..d5153b130e 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/ReachabilityWalker.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/ReachabilityWalker.java @@ -324,7 +324,13 @@ private void bfsEndBlockRoots(java.util.ArrayDeque todo) { visitScalar(value, todo); } } else if (cur instanceof RuntimeArray array) { - for (RuntimeScalar value : array.elements) { + // A child thread can run this lifecycle walk while another + // thread mutates a shared array. Iterating the live ArrayList + // leaks its fail-fast ConcurrentModificationException into + // thread teardown (seen in threads/t/free2.t). Traverse a + // point-in-time copy instead; the graph walk is advisory and + // must tolerate concurrent updates to shared Perl values. + for (RuntimeScalar value : array.elements.toArray(RuntimeScalar[]::new)) { addReachable(value, todo); visitScalar(value, todo); } @@ -373,7 +379,11 @@ private void bfs(java.util.ArrayDeque todo, boolean walkCaptures) { if (a.elements instanceof TieArray tieArray) { visitScalar(tieArray.getSelf(), todo); } - for (RuntimeScalar v : a.elements) { + // This broader weak-reference walk can also run during a + // child thread's teardown while another thread mutates a + // shared array. Never expose ArrayList's fail-fast iterator + // to concurrent Perl updates. + for (RuntimeScalar v : a.elements.toArray(RuntimeScalar[]::new)) { recordWeakArraySlotWitness(a, v); addReachable(v, todo); visitScalar(v, todo); diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeHash.java b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeHash.java index c9155e2621..d7ba5c74dd 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeHash.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeHash.java @@ -41,6 +41,12 @@ private static Stack dynamicStateStack() { public int type; // Map to store the elements of the hash public Map elements; + /** Null when ordinary hashes accept arbitrary keys; otherwise legal key set. */ + public Set legalHashKeys; + /** Values individually locked by Hash::Util. */ + public Set lockedHashValues; + /** Hash::Util lock_hash locks every current value as well as the key set. */ + public boolean allHashValuesLocked; /** True only for Perl's compile-time %^H hash. */ public boolean isHintHash; /** True for Perl's runtime %{^HOOK} hash. */ @@ -106,6 +112,32 @@ private RuntimeHashElementMap newElementMap(Map values) { return map; } + /** Enforce Hash::Util restricted-key access at every Perl hash operation. */ + public void validateHashKey(String key, String operation) { + if (legalHashKeys != null && !legalHashKeys.contains(key)) { + String message = switch (operation) { + case "delete" -> "Attempt to delete disallowed key '" + key + "' from a restricted hash"; + default -> "Attempt to access disallowed key '" + key + "' in a restricted hash"; + }; + throw new PerlCompilerException(message); + } + } + + public boolean isHashRestricted() { + return legalHashKeys != null; + } + + public boolean isHashReadOnly() { + return type == READONLY_HASH || isHashRestricted(); + } + + private void validateHashDelete(String key) { + validateHashKey(key, "delete"); + if (allHashValuesLocked || (lockedHashValues != null && lockedHashValues.contains(key))) { + throw new PerlCompilerException("Attempt to delete readonly key '" + key + "' from a restricted hash"); + } + } + void resetElementMapAfterAutovivification() { elements = newElementMap(); } @@ -462,6 +494,27 @@ public RuntimeScalar addToScalar(RuntimeScalar scalar) { * @return A RuntimeArray containing this hash (for method chaining or further operations). */ public RuntimeArray setFromList(RuntimeList value) { + if (allHashValuesLocked || type == READONLY_HASH) { + throw new PerlCompilerException("Modification of a read-only value attempted"); + } + if (legalHashKeys != null) { + Set assignedKeys = new HashSet<>(); + Iterator keys = value.iterator(); + while (keys.hasNext()) { + String key = keys.next().toString(); + validateHashKey(key, "write"); + assignedKeys.add(key); + if (keys.hasNext()) keys.next(); + } + if (lockedHashValues != null) { + for (String lockedKey : lockedHashValues) { + if (elements.containsKey(lockedKey) && !assignedKeys.contains(lockedKey)) { + throw new PerlCompilerException("Attempt to delete readonly key '" + lockedKey + + "' from a restricted hash"); + } + } + } + } return switch (type) { case PLAIN_HASH -> { // First, fully materialize the right-hand side list BEFORE clearing @@ -586,6 +639,10 @@ public RuntimeArray setFromList(RuntimeList value) { * @param value The value for the hash entry. */ public void put(String key, RuntimeScalar value) { + validateHashKey(key, "write"); + if (allHashValuesLocked || (lockedHashValues != null && lockedHashValues.contains(key))) { + throw new PerlCompilerException("Modification of a read-only value attempted"); + } validateRequireHookAssignment(key, value); switch (type) { case PLAIN_HASH -> { @@ -712,6 +769,10 @@ private static boolean isDestroyRescueAssignment( * semantics to the already-cloned value. */ public void putClonedElement(String key, RuntimeScalar value) { + validateHashKey(key, "write"); + if (allHashValuesLocked || (lockedHashValues != null && lockedHashValues.contains(key))) { + throw new PerlCompilerException("Modification of a read-only value attempted"); + } elements.put(key, value == null ? new RuntimeScalar() : value); if (value != null) { value.markContainerOwner(this); @@ -830,6 +891,7 @@ private static boolean isAsciiKey(String key) { public RuntimeScalar get(String key) { if (isEnvironmentHash) key = normalizeEnvironmentKey(key); + validateHashKey(key, "read"); if (type == TIED_HASH) { return get(new RuntimeScalar(key)); @@ -863,6 +925,7 @@ public RuntimeScalar get(String key) { */ public RuntimeScalar getForLocal(String key) { if (isEnvironmentHash) key = normalizeEnvironmentKey(key); + validateHashKey(key, "read"); if (type == TIED_HASH) { return new RuntimeTiedHashProxyEntry(this, new RuntimeScalar(key)); } @@ -890,6 +953,7 @@ public RuntimeScalar getForLocal(RuntimeScalar keyScalar) { return new RuntimeTiedHashProxyEntry(this, keyScalar); } String key = isEnvironmentHash ? normalizeEnvironmentKey(keyScalar.toString()) : keyScalar.toString(); + validateHashKey(key, "read"); boolean isByteKey = keyScalar.type == BYTE_STRING; RuntimeHashProxyEntry proxy = new RuntimeHashProxyEntry(this, key, isByteKey); RuntimeScalar existing = elements.get(key); @@ -918,6 +982,7 @@ public RuntimeScalar get(RuntimeScalar keyScalar) { return switch (this.type) { case PLAIN_HASH, AUTOVIVIFY_HASH, READONLY_HASH -> { String key = isEnvironmentHash ? normalizeEnvironmentKey(keyScalar.toString()) : keyScalar.toString(); + validateHashKey(key, "read"); var value = elements.get(key); if (value != null) { // Update the key's byte/UTF-8 flag to match the accessing key's type. @@ -983,6 +1048,7 @@ private RuntimeScalar warnForStringInterpolation(RuntimeScalar result, String ke public RuntimeScalar exists(RuntimeScalar key) { warnForUninitializedKey(key, "exists"); if (isEnvironmentHash) key = new RuntimeScalar(normalizeEnvironmentKey(key.toString())); + validateHashKey(key.toString(), "read"); return switch (type) { case PLAIN_HASH, READONLY_HASH -> new RuntimeScalar(elements.containsKey(key.toString())); // exists does not create its final key, but it does materialize a @@ -999,6 +1065,7 @@ public RuntimeScalar exists(RuntimeScalar key) { public RuntimeScalar exists(String key) { if (isEnvironmentHash) key = normalizeEnvironmentKey(key); + validateHashKey(key, "read"); return switch (type) { case PLAIN_HASH, READONLY_HASH -> new RuntimeScalar(elements.containsKey(key)); case AUTOVIVIFY_HASH -> { @@ -1012,6 +1079,7 @@ public RuntimeScalar exists(String key) { public boolean containsKey(String key) { if (isEnvironmentHash) key = normalizeEnvironmentKey(key); + validateHashKey(key, "read"); return elements.containsKey(key); } @@ -1042,6 +1110,7 @@ public RuntimeScalar deleteInContext(RuntimeScalar key, int callContext) { warnForUninitializedKey(key, "delete"); } if (isEnvironmentHash) key = new RuntimeScalar(normalizeEnvironmentKey(key.toString())); + validateHashDelete(key.toString()); return switch (type) { case PLAIN_HASH -> { String k = key.toString(); @@ -1097,6 +1166,8 @@ private static void warnForUninitializedKey(RuntimeScalar key, String operation) } public RuntimeScalar deleteInContext(String key, int callContext) { + if (isEnvironmentHash) key = normalizeEnvironmentKey(key); + validateHashDelete(key); return switch (type) { case PLAIN_HASH -> { var value = elements.remove(key); diff --git a/src/main/perl/lib/Hash/Util.pm b/src/main/perl/lib/Hash/Util.pm index 20adce4b0e..2c41080e13 100644 --- a/src/main/perl/lib/Hash/Util.pm +++ b/src/main/perl/lib/Hash/Util.pm @@ -27,31 +27,6 @@ our %EXPORT_TAGS = ( all => \@EXPORT_OK, ); -# `lock_value`/`unlock_value` lock or unlock individual hash slots. -# PerlOnJava doesn't fully model SVf_READONLY at slot granularity yet; -# tests like Storable's restrict.t need these to exist as no-ops so -# they import cleanly. Safer than refusing — most tests check for -# warnings/errors rather than that the lock-state actually persists. -# The prototype `(\%$)` auto-references the first arg, matching -# upstream's calling convention `unlock_value %hash, $key`. -sub lock_value (\%$) { return ${$_[0]}{$_[1]} } -sub unlock_value (\%$) { return ${$_[0]}{$_[1]} } - -# lock_keys_plus: like lock_keys but also pre-allocates additional -# permitted keys. We model this as a no-op too since lock_keys is -# itself best-effort. -sub lock_keys_plus (\%;@) { return $_[0] } - -# Inspector helpers — plausible defaults for tests that just need them -# to return something rather than die. -sub hashref_locked { return 0 } -sub hash_locked { return 0 } -sub hash_unlocked { return 1 } -sub legal_keys { return keys %{$_[0]} } -sub all_keys { return keys %{$_[0]} } -sub legal_ref_keys { return keys %{$_[0]} } -sub hidden_keys { return () } - 1; __END__ diff --git a/src/main/perl/lib/fields.pm b/src/main/perl/lib/fields.pm index f818d1a9b5..14eee6af11 100644 --- a/src/main/perl/lib/fields.pm +++ b/src/main/perl/lib/fields.pm @@ -46,7 +46,7 @@ sub import { my $fno = $fields->{$f}; # PerlOnJava: simplified - just record the field, skip validation. # The original checks for field reuse and module reloading are - # not needed since PerlOnJava doesn't support restricted hashes. + # omitted; fields::new uses Hash::Util to restrict instance keys. unless ($fno) { $fields->{$f} = $next; $fattr->[$next] = ($f =~ /^_/) ? PRIVATE : PUBLIC; diff --git a/src/test/resources/unit/hash_util_restricted_hash.t b/src/test/resources/unit/hash_util_restricted_hash.t new file mode 100644 index 0000000000..7a1e69d954 --- /dev/null +++ b/src/test/resources/unit/hash_util_restricted_hash.t @@ -0,0 +1,79 @@ +use strict; +use warnings; +use Test::More; +use Hash::Util qw( + lock_keys unlock_keys lock_keys_plus lock_hash unlock_hash + lock_value unlock_value hashref_locked hash_locked hash_unlocked + legal_keys hidden_keys all_keys +); + +my %hash = (present => 1); +lock_keys(%hash, qw(present permitted)); +ok(hashref_locked(\%hash), 'lock_keys marks the hash restricted'); +is_deeply([sort(legal_keys(%hash))], [qw(permitted present)], 'legal_keys includes allowed absent keys'); +is_deeply([sort(hidden_keys(%hash))], ['permitted'], 'hidden_keys reports allowed absent keys'); +ok(hash_locked(%hash), 'hash_locked recognizes restricted keys'); +ok(Internals::SvREADONLY(%hash), 'restricted hash reports readonly through SvREADONLY'); +ok(!$hash{permitted}, 'allowed absent key can be read'); +$hash{permitted} = 2; +is($hash{permitted}, 2, 'allowed absent key can be created'); +ok(grep { $_ eq 'present' } keys %hash, 'keys returns existing keys'); +my (@all_keys, @unused_keys); +my $all_keys_ref = all_keys(%hash, @all_keys, @unused_keys); +is($all_keys_ref, \%hash, 'all_keys returns the hash reference'); +is_deeply([sort @all_keys], [qw(permitted present)], 'all_keys fills the existing-key output'); +is_deeply([sort @unused_keys], [], 'all_keys fills the hidden-key output'); + +for my $operation ( + ['read', sub { my $value = $hash{forbidden} }], + ['write', sub { $hash{forbidden} = 3 }], + ['delete', sub { delete $hash{forbidden} }], +) { + my ($name, $code) = @$operation; + eval { $code->(); 1 }; + like($@, qr/Attempt to (?:access|delete) disallowed key 'forbidden' .*restricted hash/, + "$name of a disallowed key is rejected"); +} + +eval { lock_keys(%hash, 'present') }; +like($@, qr/Hash has key 'permitted' which is not in the new key set/, + 'lock_keys rejects a key set that omits an existing key'); +unlock_keys(%hash); +ok(!hashref_locked(\%hash), 'unlock_keys removes the key restriction'); +$hash{new_key} = 4; +is($hash{new_key}, 4, 'ordinary keys can be added after unlock_keys'); + +lock_keys_plus(%hash, 'extra'); +$hash{extra} = 5; +is($hash{extra}, 5, 'lock_keys_plus adds permitted keys'); +eval { my $value = $hash{not_extra} }; +like($@, qr/Attempt to access disallowed key 'not_extra'/, 'lock_keys_plus still rejects other keys'); +unlock_keys(%hash); + +lock_keys(%hash); +lock_value(%hash, 'present'); +ok(hash_locked(%hash), 'lock_value marks a value locked'); +eval { $hash{present} = 7 }; +like($@, qr/read-only value/, 'locked value cannot be assigned'); +eval { delete $hash{present} }; +like($@, qr/Attempt to delete readonly key 'present'/, 'locked value cannot be deleted'); +eval { %hash = (permitted => 6) }; +like($@, qr/Attempt to delete readonly key 'present'/, 'whole-hash assignment cannot remove a locked value'); +unlock_value(%hash, 'present'); +$hash{present} = 7; +is($hash{present}, 7, 'unlock_value restores assignment'); +ok(!hash_unlocked(%hash), 'hash_unlocked remains false while keys are restricted'); +unlock_keys(%hash); +ok(hash_unlocked(%hash), 'unlock_keys reports the hash unlocked'); + +lock_hash(%hash); +ok(hash_locked(%hash), 'lock_hash locks hash values'); +eval { my $value = $hash{missing} }; +like($@, qr/Attempt to access disallowed key 'missing'/, 'lock_hash rejects missing keys'); +eval { $hash{present} = 8 }; +like($@, qr/read-only value/, 'lock_hash prevents value assignment'); +unlock_hash(%hash); +$hash{missing} = 9; +is($hash{missing}, 9, 'unlock_hash restores ordinary hash behavior'); + +done_testing; diff --git a/src/test/resources/unit/threads_shared_array_teardown_race.t b/src/test/resources/unit/threads_shared_array_teardown_race.t new file mode 100644 index 0000000000..5efd7dd247 --- /dev/null +++ b/src/test/resources/unit/threads_shared_array_teardown_race.t @@ -0,0 +1,59 @@ +use strict; +use warnings; +use threads; +use threads::shared; + +our @roots :shared; +share(@roots); +my $ready :shared = 0; +my $start :shared = 0; + +print "1..8\n"; +my $test = 0; +sub check { + my ($condition, $name) = @_; + ++$test; + print($condition ? "ok " : "not ok ", "$test - $name\n"); +} + +my @workers; +for my $worker (1 .. 8) { + push @workers, threads->create(sub { + { + lock($ready); + ++$ready; + cond_broadcast($ready); + } + { + lock($start); + cond_wait($start) until $start; + } + + # Worker 1 exits while the other threads keep changing the shared + # array. Its per-thread END/reachability walk must not iterate the + # live ArrayList and fail with ConcurrentModificationException. + my $iterations = $worker == 1 ? 10 : 3000; + for (1 .. $iterations) { + lock(@roots); + push @roots, $_; + shift @roots if @roots > 16; + } + return $worker; + }); +} + +{ + lock($ready); + cond_wait($ready) until $ready == 8; +} +{ + lock($start); + $start = 1; + cond_broadcast($start); +} + +for my $worker (1 .. 8) { + my $result = eval { $workers[$worker - 1]->join() }; + check(!$@ && defined($result) && $result == $worker, + "worker $worker exits cleanly during shared-array mutation"); +}