This is an automated email from the ASF dual-hosted git repository.
slawrence pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/daffodil.git
The following commit(s) were added to refs/heads/main by this push:
new 0f5ebcab7 Fix deadlock issue with custom Enum implementation
0f5ebcab7 is described below
commit 0f5ebcab73bc54394e58597b6e44d36ddfbfd8ee
Author: Steve Lawrence <[email protected]>
AuthorDate: Wed Jun 15 14:45:01 2022 -0400
Fix deadlock issue with custom Enum implementation
Our Enum implementation use for property values/lookups has important
benefits over Scala Enumerations. However, it seems like the way we
implement it can lead to circular deadlocks. When a Value is initialized
it causes the Enum to be initialized. And the Enum initialized calls
forceConstruction to initialized all the Values. This leads to circular
instantiating that works in most cases, but sometimes leads to a
deadlock. Where this deadlock exists or how to fix it is not clear. It's
also not clear what changed to make this deadlock much more likely.
To avoid this, we this change removes the forceConstruction function and
replaces it with a manually managed Array of Enum Values. This Array is
lazily evaluated so that instantiated an Enum does not also directly
instantiate the Enum Value, avoiding the circular deadlock. This does
require extra code to define an Enum, but the majority of Enums are in
generated code so doesn't add much maintenance burden.
DAFFODIL-2704
---
.../src/main/scala/org/apache/daffodil/Main.scala | 4 +--
.../annotation/props/TestPropertyRuntime.scala | 13 +++----
.../schema/annotation/props/ByHandMixins.scala | 16 +++++----
.../schema/annotation/props/Properties.scala | 40 ++++++----------------
.../annotation/props/TestGeneratedProperties.scala | 2 +-
.../daffodil/propGen/PropertyGenerator.scala | 12 +++----
.../apache/daffodil/propGen/TunableGenerator.scala | 9 +++--
.../apache/daffodil/propGen/WarnIDGenerator.scala | 10 ++++--
.../daffodil/layers/LineFoldedTransformer.scala | 5 +--
.../org/apache/daffodil/tdml/RunnerFactory.scala | 2 +-
10 files changed, 55 insertions(+), 58 deletions(-)
diff --git a/daffodil-cli/src/main/scala/org/apache/daffodil/Main.scala
b/daffodil-cli/src/main/scala/org/apache/daffodil/Main.scala
index 7002d5b74..8f97d357f 100644
--- a/daffodil-cli/src/main/scala/org/apache/daffodil/Main.scala
+++ b/daffodil-cli/src/main/scala/org/apache/daffodil/Main.scala
@@ -204,7 +204,7 @@ class CLIConf(arguments: Array[String]) extends
scallop.ScallopConf(arguments) {
val optImplementation =
TDMLImplementation.optionStringToEnum("implementation", s)
if (!optImplementation.isDefined) {
throw new Exception("Unrecognized TDML implementation '%s'. Must be one
of %s"
- .format(s, TDMLImplementation.allValues.mkString(", ")))
+ .format(s, TDMLImplementation.values.mkString(", ")))
}
optImplementation.get
})
@@ -409,7 +409,7 @@ class CLIConf(arguments: Array[String]) extends
scallop.ScallopConf(arguments) {
val implementation = opt[TDMLImplementation](short = 'I', argName =
"implementation",
descr = "Implementation to run TDML tests. Choose one of %s. Defaults to
%s."
- .format(TDMLImplementation.allValues.mkString(", "),
TDMLImplementation.Daffodil.toString),
+ .format(TDMLImplementation.values.mkString(", "),
TDMLImplementation.Daffodil.toString),
default = None)
val info = tally(descr = "Increment test result information output level,
one level for each -i")
val list = opt[Boolean](descr = "Show names and descriptions instead of
running test cases")
diff --git
a/daffodil-core/src/test/scala/org/apache/daffodil/schema/annotation/props/TestPropertyRuntime.scala
b/daffodil-core/src/test/scala/org/apache/daffodil/schema/annotation/props/TestPropertyRuntime.scala
index 9a2374af7..036f2769b 100644
---
a/daffodil-core/src/test/scala/org/apache/daffodil/schema/annotation/props/TestPropertyRuntime.scala
+++
b/daffodil-core/src/test/scala/org/apache/daffodil/schema/annotation/props/TestPropertyRuntime.scala
@@ -30,9 +30,9 @@ object MyProp extends Enum[MyPropType] // with ThrowsSDE
lazy val context = Fakes.fakeElem
// lazy val schemaComponent = context
case object PropVal1 extends MyPropType
- forceConstruction(PropVal1)
case object PropVal2 extends MyPropType
- forceConstruction(PropVal2)
+ override lazy val values = Array(PropVal1, PropVal2)
+
def apply(name: String): MyPropType = apply(name, context)
def apply(name: String, context: ThrowsSDE) = stringToEnum("myProp", name,
context)
}
@@ -48,7 +48,7 @@ class TestPropertyRuntime {
@Test
def testConstructed(): Unit = {
// val myPropUser = new RealObject
- val av = MyProp.allValues
+ val av = MyProp.values
val pv1 = MyProp.PropVal1
val pv2 = MyProp.PropVal2
assertTrue(av.contains(pv1))
@@ -85,9 +85,10 @@ class HasMixin extends SchemaComponentImpl(<foo/>, None)
sealed trait TheExampleProp extends TheExampleProp.Value
object TheExampleProp extends Enum[TheExampleProp] {
- case object Left extends TheExampleProp; forceConstruction(Left)
- case object Right extends TheExampleProp; forceConstruction(Right)
- case object Center extends TheExampleProp; forceConstruction(Center)
+ case object Left extends TheExampleProp
+ case object Right extends TheExampleProp
+ case object Center extends TheExampleProp
+ override lazy val values = Array(Left, Right, Center)
def apply(name: String, self: ThrowsSDE): TheExampleProp =
stringToEnum("theExampleProp", name, self)
}
diff --git
a/daffodil-lib/src/main/scala/org/apache/daffodil/schema/annotation/props/ByHandMixins.scala
b/daffodil-lib/src/main/scala/org/apache/daffodil/schema/annotation/props/ByHandMixins.scala
index 5318578b5..93f99ea54 100644
---
a/daffodil-lib/src/main/scala/org/apache/daffodil/schema/annotation/props/ByHandMixins.scala
+++
b/daffodil-lib/src/main/scala/org/apache/daffodil/schema/annotation/props/ByHandMixins.scala
@@ -58,6 +58,8 @@ import passera.unsigned.ULong
sealed trait AlignmentType extends AlignmentType.Value
object AlignmentType extends Enum[AnyRef] { // Note: Was using AlignmentUnits
mixin here!
case object Implicit extends AlignmentType
+ override lazy val values = Array(Implicit)
+
val allowedAlignmentValues = {
val ints = 0 to 30 // that's every perfect power of 2 that fits in an Int.
ints.map(1 << _)
@@ -126,10 +128,11 @@ trait TextStandardBaseMixin extends PropertyMixin {
sealed trait SeparatorSuppressionPolicy extends
SeparatorSuppressionPolicy.Value
object SeparatorSuppressionPolicy extends Enum[SeparatorSuppressionPolicy] {
- case object Never extends SeparatorSuppressionPolicy;
forceConstruction(Never)
- case object TrailingEmpty extends SeparatorSuppressionPolicy;
forceConstruction(TrailingEmpty)
- case object TrailingEmptyStrict extends SeparatorSuppressionPolicy;
forceConstruction(TrailingEmptyStrict)
- case object AnyEmpty extends SeparatorSuppressionPolicy;
forceConstruction(AnyEmpty)
+ case object Never extends SeparatorSuppressionPolicy
+ case object TrailingEmpty extends SeparatorSuppressionPolicy
+ case object TrailingEmptyStrict extends SeparatorSuppressionPolicy
+ case object AnyEmpty extends SeparatorSuppressionPolicy
+ override lazy val values = Array(Never, TrailingEmpty, TrailingEmptyStrict,
AnyEmpty)
def apply(name: String, self: ThrowsSDE): SeparatorSuppressionPolicy =
stringToEnum("separatorSuppressionPolicy", name, self)
}
@@ -427,8 +430,9 @@ trait TextStandardExponentRepMixin extends PropertyMixin {
*/
sealed trait EmptyElementParsePolicy extends EmptyElementParsePolicy.Value
object EmptyElementParsePolicy extends Enum[EmptyElementParsePolicy] {
- case object TreatAsMissing extends EmptyElementParsePolicy;
forceConstruction(TreatAsMissing)
- case object TreatAsEmpty extends EmptyElementParsePolicy;
forceConstruction(TreatAsEmpty)
+ case object TreatAsMissing extends EmptyElementParsePolicy
+ case object TreatAsEmpty extends EmptyElementParsePolicy
+ override lazy val values = Array(TreatAsMissing, TreatAsEmpty)
def apply(name: String, context: ThrowsSDE): EmptyElementParsePolicy =
stringToEnum("emptyElementParsePolicy", name, context)
}
diff --git
a/daffodil-lib/src/main/scala/org/apache/daffodil/schema/annotation/props/Properties.scala
b/daffodil-lib/src/main/scala/org/apache/daffodil/schema/annotation/props/Properties.scala
index 038c3c7c4..19a6bc90b 100644
---
a/daffodil-lib/src/main/scala/org/apache/daffodil/schema/annotation/props/Properties.scala
+++
b/daffodil-lib/src/main/scala/org/apache/daffodil/schema/annotation/props/Properties.scala
@@ -21,7 +21,6 @@ import org.apache.daffodil.exceptions._
import org.apache.daffodil.util.Misc._
import org.apache.daffodil.util._
import org.apache.daffodil.cookers.Converter
-import scala.collection.mutable
/**
* Enum class as basis for our DFDL properties
@@ -46,9 +45,10 @@ import scala.collection.mutable
*
* sealed trait BinaryNumberRep extends BinaryNumberRep.Value
* object BinaryNumberRep extends Enum[BinaryNumberRep] {
- * case object Packed extends BinaryNumberRep ; forceConstruction(Packed)
- * case object Bcd extends BinaryNumberRep ; forceConstruction(Bcd)
- * case object Binary extends BinaryNumberRep ; forceConstruction(Binary)
+ * case object Packed extends BinaryNumberRep
+ * case object Bcd extends BinaryNumberRep
+ * case object Binary extends BinaryNumberRep
+ * override lazy val values = Array(Packed, Bcd, Binary)
*
* def apply(name: String) : BinaryNumberRep =
stringToEnum("binaryNumberRep", name)
* }
@@ -84,7 +84,8 @@ import scala.collection.mutable
abstract class EnumBase
abstract class EnumValueBase extends Serializable
abstract class Enum[A] extends EnumBase with Converter[String, A] {
- class Value extends EnumValueBase { self: A => {
+ class Value extends EnumValueBase { self: A =>
+ override lazy val toString = {
val theVal = this
val cn = getNameFromClass(this)
val en = cn match {
@@ -94,18 +95,13 @@ abstract class Enum[A] extends EnumBase with
Converter[String, A] {
case "Sunday" | "Monday" | "Tuesday" | "Wednesday" | "Thursday" |
"Friday" | "Saturday" => cn
case _ => Misc.toInitialLowerCaseUnlessAllUpperCase(cn)
}
- _values += (en -> theVal)
- _values
- }
- override lazy val toString = {
- val propName =
Misc.toInitialLowerCaseUnlessAllUpperCase(getNameFromClass(this))
- propName
+ en
}
}
def toPropName(prop: A) = prop.toString
- private val _values = mutable.ArrayBuffer.empty[Tuple2[String, A]]
+ val values: Array[A]
/**
* This is invoked at runtime to compare expression results to see if they
@@ -121,29 +117,15 @@ abstract class Enum[A] extends EnumBase with
Converter[String, A] {
def optionStringToEnum(enumTypeName: String, str: String): Option[A] = {
var i: Int = 0
- while (i < _values.size) {
- if (_values(i)._1.equals(str)) { // was equals ignore case - that's just
going to allow errors if used at runtime
- return Some(_values(i)._2)
+ while (i < values.size) {
+ if (values(i).toString.equals(str)) {
+ return Some(values(i))
}
i += 1
}
None
}
- /**
- * Useful for diagnostic messages where you want to say "must be one of
...." and list the possibilities.
- */
- def allValues = _values.map { case (k, v) => v }
-
- /**
- * Scala delays construction of case objects (presumably because many
programs don't use them at all)
- * We need to force creation of our inner property case objects because
constructing them also has
- * the side-effect of registering them in the _values list.
- */
- def forceConstruction(obj: Any): Unit = {
- //Assert.invariant(obj.toString() != "") // TODO: is forceConstruction
needed at all?
- }
-
override protected def convert(b: String, context: ThrowsSDE, forUnparse:
Boolean): A = apply(b, context)
def apply(name: String, context: ThrowsSDE): A
diff --git
a/daffodil-lib/src/test/scala/org/apache/daffodil/schema/annotation/props/TestGeneratedProperties.scala
b/daffodil-lib/src/test/scala/org/apache/daffodil/schema/annotation/props/TestGeneratedProperties.scala
index e1bcaaa86..a80ff44a7 100644
---
a/daffodil-lib/src/test/scala/org/apache/daffodil/schema/annotation/props/TestGeneratedProperties.scala
+++
b/daffodil-lib/src/test/scala/org/apache/daffodil/schema/annotation/props/TestGeneratedProperties.scala
@@ -167,7 +167,7 @@ class TestGeneratedProperties {
comparePropValue(hasProps.calendarCheckPolicy, "lax")
comparePropValue(hasProps.calendarTimeZone, "UTC")
comparePropValue(hasProps.calendarObserveDST, "yes")
- comparePropValue(hasProps.calendarFirstDayOfWeek, "monday")
+ comparePropValue(hasProps.calendarFirstDayOfWeek, "Monday")
comparePropValue(hasProps.calendarDaysInFirstWeek, "4")
comparePropValue(hasProps.calendarCenturyStart, "53")
comparePropValue(hasProps.occursCountKind, "parsed")
diff --git
a/daffodil-propgen/src/main/scala/org/apache/daffodil/propGen/PropertyGenerator.scala
b/daffodil-propgen/src/main/scala/org/apache/daffodil/propGen/PropertyGenerator.scala
index 747e70e56..283c965fe 100644
---
a/daffodil-propgen/src/main/scala/org/apache/daffodil/propGen/PropertyGenerator.scala
+++
b/daffodil-propgen/src/main/scala/org/apache/daffodil/propGen/PropertyGenerator.scala
@@ -340,7 +340,7 @@ class PropertyGenerator(arg: Node) {
object Currency extends Enum[Currency] {
"""
val templateMiddle =
- """ case object EUR extends Currency ; forceConstruction(EUR)
+ """ case object EUR extends Currency
"""
// Modified to pass the context, so diagnostics can be better.
@@ -425,11 +425,9 @@ trait CurrencyMixin extends PropertyMixin {
val traitName = initialUpperCase(pname)
val propName = initialLowerCase(pname)
val middle = templateMiddle.replaceAll("Currency", traitName)
- val mids = pvalues.map(pvalue => {
- // need to insure the enum values aren't digits (as in the
TextNumberBase enum)
- val pvalueAsIdentifier = if (pvalue.charAt(0).isDigit) "INTEGER_" +
pvalue else pvalue
- middle.replace("EUR", initialUpperCase(pvalueAsIdentifier))
- })
+ val pvalueIDs = pvalues.map(pvalue => initialUpperCase(pvalue))
+ val mids = pvalueIDs.map(id => middle.replace("EUR", id))
+ val values = pvalueIDs.mkString(" override lazy val values = Array(", ",
", ")\n")
val start = templateStart.replaceAll("Currency", traitName)
val end = templateEnd.replaceAll("Currency",
traitName).replaceAll("currency", propName)
val mixin =
@@ -437,7 +435,7 @@ trait CurrencyMixin extends PropertyMixin {
else {
templateMixin.replaceAll("Currency", traitName).replaceAll("currency",
propName)
}
- val res = start + mids.foldLeft("")(_ + _) + end + mixin
+ val res = start + mids.foldLeft("")(_ + _) + values + end + mixin
res
}
diff --git
a/daffodil-propgen/src/main/scala/org/apache/daffodil/propGen/TunableGenerator.scala
b/daffodil-propgen/src/main/scala/org/apache/daffodil/propGen/TunableGenerator.scala
index 8dd9e802d..108181433 100644
---
a/daffodil-propgen/src/main/scala/org/apache/daffodil/propGen/TunableGenerator.scala
+++
b/daffodil-propgen/src/main/scala/org/apache/daffodil/propGen/TunableGenerator.scala
@@ -351,7 +351,12 @@ class TunableEnumDefinition(schemaRootConfig:
scala.xml.Node, schemaRootExt: sca
private val scalaEnums = {
val scalaEnumValues = allEnumerationValues.map { e => e.head.toUpper +
e.tail }
- scalaEnumValues.map { e => s""" case object ${e} extends ${scalaType};
forceConstruction(${e})""" }
+ scalaEnumValues.map { e => s""" case object ${e} extends ${scalaType}""" }
+ }
+
+ private val values = {
+ val scalaEnumValues = allEnumerationValues.map { e => e.head.toUpper +
e.tail }
+ scalaEnumValues.mkString(" override lazy val values = Array(", ", ", ")")
}
private val bottom = s"""
@@ -360,7 +365,7 @@ class TunableEnumDefinition(schemaRootConfig:
scala.xml.Node, schemaRootExt: sca
""".stripMargin
val scalaEnumeration = {
- top + "\n" + scalaEnums.mkString("\n") + "\n" + bottom
+ top + "\n" + scalaEnums.mkString("\n") + "\n" + values + "\n" + bottom
}
}
diff --git
a/daffodil-propgen/src/main/scala/org/apache/daffodil/propGen/WarnIDGenerator.scala
b/daffodil-propgen/src/main/scala/org/apache/daffodil/propGen/WarnIDGenerator.scala
index 3aafbd674..e890f7429 100644
---
a/daffodil-propgen/src/main/scala/org/apache/daffodil/propGen/WarnIDGenerator.scala
+++
b/daffodil-propgen/src/main/scala/org/apache/daffodil/propGen/WarnIDGenerator.scala
@@ -71,12 +71,18 @@ class WarnIDGenerator(schema: scala.xml.Node) {
w.write(top)
w.write("\n")
- enumerationNodes.foreach { node =>
+ val scalaNames = enumerationNodes.map { node =>
val enumName = node \@ "value"
val scalaName = enumName.head.toUpper + enumName.tail
- w.write(s" case object ${scalaName} extends WarnID;
forceConstruction($scalaName)\n")
+ scalaName
}
+ scalaNames.foreach { scalaName =>
+ w.write(s" case object ${scalaName} extends WarnID\n")
+ }
+
+ w.write(scalaNames.mkString(" override lazy val values = Array(", ", ",
")\n"))
+
w.write("\n")
w.write(bottom)
w.flush();
diff --git
a/daffodil-runtime1-layers/src/main/scala/org/apache/daffodil/layers/LineFoldedTransformer.scala
b/daffodil-runtime1-layers/src/main/scala/org/apache/daffodil/layers/LineFoldedTransformer.scala
index f182c45e8..0aef89853 100644
---
a/daffodil-runtime1-layers/src/main/scala/org/apache/daffodil/layers/LineFoldedTransformer.scala
+++
b/daffodil-runtime1-layers/src/main/scala/org/apache/daffodil/layers/LineFoldedTransformer.scala
@@ -126,8 +126,9 @@ sealed trait LineFoldMode extends LineFoldMode.Value {
object LineFoldMode extends Enum[LineFoldMode] {
- case object IMF extends LineFoldMode; forceConstruction(Left)
- case object iCalendar extends LineFoldMode; forceConstruction(Right)
+ case object IMF extends LineFoldMode
+ case object iCalendar extends LineFoldMode
+ override lazy val values = Array(IMF, iCalendar)
override def apply(name: String, context: ThrowsSDE): LineFoldMode =
stringToEnum("lineFoldMode", name, context)
}
diff --git
a/daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/RunnerFactory.scala
b/daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/RunnerFactory.scala
index 8d230016c..0f8bc4ab9 100644
---
a/daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/RunnerFactory.scala
+++
b/daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/RunnerFactory.scala
@@ -98,7 +98,7 @@ object Runner {
* A test or test suite can override this to specify more or different
implementations
* that the test should pass for.
*/
- def defaultImplementationsDefaultDefault =
TDMLImplementation.allValues.map(_.toString)
+ def defaultImplementationsDefaultDefault =
TDMLImplementation.values.map(_.toString)
/**
* By default we don't run Daffodil negative TDML tests against
cross-testers.
