getSingleLineStringWriter: Use try-finally, and only one stringWriter#16751
Conversation
ghost
commented
Jun 26, 2017
ghost
commented
- Failure to use try-with-resources can lead to state persisting after an exception. Could be the cause of this.
- If we correctly clean up the StringWriter, then we should only need one global instance, not an array.
ghost
force-pushed
the
singleLineStringWriter
branch
from
7ce90a2 to
6023898
Compare
ghost
mentioned this pull request
ghost
force-pushed
the
singleLineStringWriter
branch
2 times, most recently
from
0c32972 to
9caaf17
Compare
ghost
force-pushed
the
singleLineStringWriter
branch
from
9caaf17 to
8ac59a9
Compare
sandersn
left a comment
sandersn
left a comment
There was a problem hiding this comment.
Questions about whether the affected functions can be called simultaneously.
| } | ||
|
|
||
| function typePredicateToString(typePredicate: TypePredicate, enclosingDeclaration?: Declaration, flags?: TypeFormatFlags): string { | ||
| const writer = getSingleLineStringWriter(); |
There was a problem hiding this comment.
I take it this is the only place that getSingleLineStringWriter was used? I wonder why it had the complex pooling behaviour then. Do you know whether it was used in more locations in the past?
There was a problem hiding this comment.
Can these functions be called multiple times at once? In other words, do they each need to have their own copy of a string writer instead of a singleton?
There was a problem hiding this comment.
That would only happen if one of these 3 functions (symbolToString, signatureToString, typePredicateToString) called another one. I added an assert to prevent us from doing that.
2 participants
