Skip to content

Fix deadlock issue with custom Enum implementation#802

Merged
stevedlawrence merged 1 commit intoapache:mainfrom
stevedlawrence:daffodil-2704-enum-deadlock
Jun 16, 2022
Merged

Fix deadlock issue with custom Enum implementation#802
stevedlawrence merged 1 commit intoapache:mainfrom
stevedlawrence:daffodil-2704-enum-deadlock

Conversation

@stevedlawrence
Copy link
Member

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

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
Copy link
Contributor

@tuxji tuxji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Thanks for fixing this issue.

Copy link
Contributor

@mbeckerle mbeckerle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.
+1

@stevedlawrence stevedlawrence merged commit 0f5ebca into apache:main Jun 16, 2022
@stevedlawrence stevedlawrence deleted the daffodil-2704-enum-deadlock branch June 16, 2022 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants