On 13/06/2019 20:18, Andrew Leonard wrote:
Thanks Alex, good spot.
I have done all the changes now with the latest updates, and have
performed some stress tests on xLinux, Win64 and Mac64, all successful.
Please can I get reviews and sponsor for webrev.04:
http://cr.openjdk.java.net/~aleonard/8225474/webrev.04/
Retrofitting code that wasn't developed to be used by multiple
concurrent threads to be thread safe takes time and will require a lot
of review effort. I don't have time to spend on this now and I suspect
this one will need a second reviewer to help with the confidence that
you've got everything.
Things to check are:
- ConnectorImpl::toString iterates over the values in defaultArguments,
I assume this will require synchronization
- ConnectorImpl.trueString and falseString are initialized lazily
without synchronization. It might be okay but a comment to explain why.
Ditto for the lazily initialization of the messages field in getString.
- All but the "value" field in the Connector.Argument implementations
should be final. It might be that the value fields need to volatile
(needs more study).
- GenericListeningConnector fields should be final. You can also
initialize listenMap with new ConcurrentHashMap<>() .
- GenericListeningConnector.accept needs further analysis, at least for
the case where the debugger hasn't called startListening.
There may be more issues, it just needs time to walk through the
original implementation.
-Alan
