[
https://issues.apache.org/jira/browse/DAFFODIL-3093?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Olabusayo Kilo updated DAFFODIL-3093:
-------------------------------------
Description:
_DirectOrBufferedDataOutputStream_ models a forward-only linked chain via a
private _{_}following: Maybe[DirectOrBufferedDataOutputStream]{_} field:
splitting a stream ({_}addBuffered(){_}/{_}addBufferedDOS{_}) sets
_{_}following{_} on the *old* node to point at the *new* node, and every call
site that performs a split immediately re-points the owning {_}UState{_}'s
"current" DOS reference ({_}setDataOutputStream{_}) at that new node in the
same step (see {_}Suspension.scala{_}'s {_}prepareToSuspend{_}/{_}splitDOS{_},
{_}LayeredSequenceUnparser.scala{_}, {_}UState.scala{_}'s bit-order-change
split, {_}BlobLengthUnparser.scala{_}). As a result, the state's "current"
pointer always tracks the newest node in the chain which itself never has
_{_}following{_} set (nothing has split off of it yet).
_cleanUp()_ ({_}DirectOrBufferedDataOutputStream.scala{_}) is only ever invoked
via that same "current" pointer, from two call sites in
{_}DataProcessor.scala{_}'s _finally_ blocks ({_}unparseSinglePass{_},
{_}unparseViaBuildThenWrite{_}). It does two things:
1. Deletes _this_ object's own temp file, if it is currently buffering to one.
2. Recurses forward through _{_}following{_}, repeating step 1 on each
successor.
Because _cleanUp()_ always starts at the newest, and _{_}following{_} only
points forward, step 2 can never reach a node *before* the newest in the chain.
In the normal/successful-completion case this doesn't matter: as each
suspension resolves, {_}setFinished(){_}'s own forward-merge cascade
({_}deliverContent{_}, which copies a buffered node's bytes into the direct
stream and deletes its temp file as part of that) collapses the whole chain
down to one node before _cleanUp()_ ever runs. But if the unparse aborts
*before* every pending split has been merged, ex. two or more suspensions are
permanently blocked (a genuine deadlock), any split that (a) already spilled to
a temp file and (b) is not the newest and was never merged, is unreachable by
either mechanism. Its temp file is simply never deleted by Daffodil; it
survives only because _File.createTempFile(...).deleteOnExit()_ happens to be
set at file-creation time ({_}ByteArrayOrFileOutputStream.checkBuffer{_}),
which cleans it up at JVM exit, not at unparse-failure time.
To reproduce:
Schema with two _dfdl:outputValueCalc_ elements, each referencing a distinct
_dfdl:defineVariable_ that has no default value and is never set (so each
creates a _Suspension_ that can never resolve , a real, permanent deadlock, not
a timing artifact), with ordinary content in between:
{code:xml}
<dfdl:defineVariable name="neverSet1" type="xs:int"/>
<dfdl:defineVariable name="neverSet2" type="xs:int"/>
...
<xs:element name="computed1" type="xs:int" dfdl:lengthKind="explicit"
dfdl:length="4"
dfdl:outputValueCalc="{ $tns:neverSet1 }"/>
<xs:element name="middle" type="xs:string" dfdl:lengthKind="explicit"
dfdl:length="40"/>
<xs:element name="computed2" type="xs:int" dfdl:lengthKind="explicit"
dfdl:length="4"
dfdl:outputValueCalc="{ $tns:neverSet2 }"/>
<xs:element name="filler" type="xs:string" dfdl:lengthKind="explicit"
dfdl:length="40"/>
{code}
Compiled with _maxByteArrayOutputStreamBufferSizeInBytes_ set very small (e.g.
_1_) so that both buffered splits (behind _computed1_ and behind _computed2_)
immediately spill to temp files instead of staying in memory, and
_tempFilePath_ pointed at a scratch directory so leftover files are easy to
observe.
Unparsing this infoset:
1. Correctly fails with _SuspensionDeadlockException_ ("Expressions/Unparsers
are circularly deadlocked").
2. Creates two temp files (one behind _computed1_, holding _middle_'s bytes;
one behind _computed2_, holding _filler_'s bytes, the newest at the time of
failure).
3. Deletes exactly one of them (the newest's, via _cleanUp()_'s local check).
The other (behind _computed1_) is left on disk.
was:
_DirectOrBufferedDataOutputStream_ models a forward-only linked chain via a
private _{_}following: Maybe[DirectOrBufferedDataOutputStream]{_} field:
splitting
a stream ({_}addBuffered(){_}/{_}addBufferedDOS{_}) sets _{_}following{_} on
the *old*
node to point at the *new* node, and every call site that performs a split
immediately re-points the owning {_}UState{_}'s "current" DOS reference
({_}setDataOutputStream{_}) at that new node in the same step (see
{_}Suspension.scala{_}'s {_}prepareToSuspend{_}/{_}splitDOS{_},
{_}LayeredSequenceUnparser.scala{_},
{_}UState.scala{_}'s bit-order-change split, {_}BlobLengthUnparser.scala{_}).
As a
result, the state's "current" pointer always tracks the newest node in the
chain which itself never has _{_}following{_} set (nothing has
split off of it yet).
_cleanUp()_ ({_}DirectOrBufferedDataOutputStream.scala{_}) is only
ever invoked via that same "current" pointer, from two call sites in
{_}DataProcessor.scala{_}'s _finally_ blocks ({_}unparseSinglePass{_},
{_}unparseViaBuildThenWrite{_}). It does two things:
1. Deletes _this_ object's own temp file, if it is currently buffering to one.
2. Recurses forward through _{_}following{_}, repeating step 1 on each
successor.
Because _cleanUp()_ always starts at the newest, and _{_}following{_} only
points forward, step 2 can never reach a node *before* the newest in the
chain. In the normal/successful-completion case this doesn't matter: as each
suspension resolves, {_}setFinished(){_}'s own forward-merge cascade
({_}deliverContent{_}, which copies a buffered node's bytes into the direct
stream and deletes its temp file as part of that) collapses the whole chain
down to one node before _cleanUp()_ ever runs. But if the unparse aborts
*before* every pending split has been merged, ex. two or more suspensions
are permanently blocked (a genuine deadlock), any split that (a) already
spilled to a temp file and (b) is not the newest and was never merged, is
unreachable by either mechanism. Its temp file is simply never deleted by
Daffodil; it survives only because _File.createTempFile(...).deleteOnExit()_
happens to be set at file-creation time
({_}ByteArrayOrFileOutputStream.checkBuffer{_}), which cleans it up at JVM exit,
not at unparse-failure time.
To reproduce:
Schema with two _dfdl:outputValueCalc_ elements, each referencing a distinct
_dfdl:defineVariable_ that has no default value and is never set (so each
creates a _Suspension_ that can never resolve , a real, permanent deadlock,
not a timing artifact), with ordinary content in between:
{code:xml}
<dfdl:defineVariable name="neverSet1" type="xs:int"/>
<dfdl:defineVariable name="neverSet2" type="xs:int"/>
...
<xs:element name="computed1" type="xs:int" dfdl:lengthKind="explicit"
dfdl:length="4"
dfdl:outputValueCalc="{ $tns:neverSet1 }"/>
<xs:element name="middle" type="xs:string" dfdl:lengthKind="explicit"
dfdl:length="40"/>
<xs:element name="computed2" type="xs:int" dfdl:lengthKind="explicit"
dfdl:length="4"
dfdl:outputValueCalc="{ $tns:neverSet2 }"/>
<xs:element name="filler" type="xs:string" dfdl:lengthKind="explicit"
dfdl:length="40"/>
{code}
Compiled with _maxByteArrayOutputStreamBufferSizeInBytes_ set very small
(e.g. _1_) so that both buffered splits (behind _computed1_ and behind
_computed2_) immediately spill to temp files instead of staying in memory,
and _tempFilePath_ pointed at a scratch directory so leftover files are easy
to observe.
Unparsing this infoset:
1. Correctly fails with _SuspensionDeadlockException_ ("Expressions/Unparsers
are circularly deadlocked").
2. Creates two temp files (one behind _computed1_, holding _middle_'s bytes;
one behind _computed2_, holding _filler_'s bytes, the newest at the
time of failure).
3. Deletes exactly one of them (the newest's, via _cleanUp()_'s local
check). The other (behind _computed1_) is left on disk.
> DirectOrBufferedDataOutputStream.cleanUp() cannot reach temp files from
> upstream, un-merged buffered splits
> -----------------------------------------------------------------------------------------------------------
>
> Key: DAFFODIL-3093
> URL: https://issues.apache.org/jira/browse/DAFFODIL-3093
> Project: Daffodil
> Issue Type: Bug
> Components: Back End, Performance
> Affects Versions: 4.2.0
> Reporter: Olabusayo Kilo
> Priority: Minor
>
> _DirectOrBufferedDataOutputStream_ models a forward-only linked chain via a
> private _{_}following: Maybe[DirectOrBufferedDataOutputStream]{_} field:
> splitting a stream ({_}addBuffered(){_}/{_}addBufferedDOS{_}) sets
> _{_}following{_} on the *old* node to point at the *new* node, and every call
> site that performs a split immediately re-points the owning {_}UState{_}'s
> "current" DOS reference ({_}setDataOutputStream{_}) at that new node in the
> same step (see {_}Suspension.scala{_}'s
> {_}prepareToSuspend{_}/{_}splitDOS{_}, {_}LayeredSequenceUnparser.scala{_},
> {_}UState.scala{_}'s bit-order-change split, {_}BlobLengthUnparser.scala{_}).
> As a result, the state's "current" pointer always tracks the newest node in
> the chain which itself never has _{_}following{_} set (nothing has split off
> of it yet).
> _cleanUp()_ ({_}DirectOrBufferedDataOutputStream.scala{_}) is only ever
> invoked via that same "current" pointer, from two call sites in
> {_}DataProcessor.scala{_}'s _finally_ blocks ({_}unparseSinglePass{_},
> {_}unparseViaBuildThenWrite{_}). It does two things:
> 1. Deletes _this_ object's own temp file, if it is currently buffering to one.
> 2. Recurses forward through _{_}following{_}, repeating step 1 on each
> successor.
> Because _cleanUp()_ always starts at the newest, and _{_}following{_} only
> points forward, step 2 can never reach a node *before* the newest in the
> chain. In the normal/successful-completion case this doesn't matter: as each
> suspension resolves, {_}setFinished(){_}'s own forward-merge cascade
> ({_}deliverContent{_}, which copies a buffered node's bytes into the direct
> stream and deletes its temp file as part of that) collapses the whole chain
> down to one node before _cleanUp()_ ever runs. But if the unparse aborts
> *before* every pending split has been merged, ex. two or more suspensions are
> permanently blocked (a genuine deadlock), any split that (a) already spilled
> to a temp file and (b) is not the newest and was never merged, is unreachable
> by either mechanism. Its temp file is simply never deleted by Daffodil; it
> survives only because _File.createTempFile(...).deleteOnExit()_ happens to be
> set at file-creation time ({_}ByteArrayOrFileOutputStream.checkBuffer{_}),
> which cleans it up at JVM exit, not at unparse-failure time.
> To reproduce:
> Schema with two _dfdl:outputValueCalc_ elements, each referencing a distinct
> _dfdl:defineVariable_ that has no default value and is never set (so each
> creates a _Suspension_ that can never resolve , a real, permanent deadlock,
> not a timing artifact), with ordinary content in between:
> {code:xml}
> <dfdl:defineVariable name="neverSet1" type="xs:int"/>
> <dfdl:defineVariable name="neverSet2" type="xs:int"/>
> ...
> <xs:element name="computed1" type="xs:int" dfdl:lengthKind="explicit"
> dfdl:length="4"
> dfdl:outputValueCalc="{ $tns:neverSet1 }"/>
> <xs:element name="middle" type="xs:string" dfdl:lengthKind="explicit"
> dfdl:length="40"/>
> <xs:element name="computed2" type="xs:int" dfdl:lengthKind="explicit"
> dfdl:length="4"
> dfdl:outputValueCalc="{ $tns:neverSet2 }"/>
> <xs:element name="filler" type="xs:string" dfdl:lengthKind="explicit"
> dfdl:length="40"/>
> {code}
> Compiled with _maxByteArrayOutputStreamBufferSizeInBytes_ set very small
> (e.g. _1_) so that both buffered splits (behind _computed1_ and behind
> _computed2_) immediately spill to temp files instead of staying in memory,
> and _tempFilePath_ pointed at a scratch directory so leftover files are easy
> to observe.
> Unparsing this infoset:
> 1. Correctly fails with _SuspensionDeadlockException_ ("Expressions/Unparsers
> are circularly deadlocked").
> 2. Creates two temp files (one behind _computed1_, holding _middle_'s bytes;
> one behind _computed2_, holding _filler_'s bytes, the newest at the time of
> failure).
> 3. Deletes exactly one of them (the newest's, via _cleanUp()_'s local check).
> The other (behind _computed1_) is left on disk.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
