|[dsdp-tm-dev] Re: DStoreConnectorService v1.57 is at risk of deadlock|
21/02/2008 05:31 AM
protected synchronized void internalConnect(IProgressMonitor monitor) throws Exception
making this method synchronized is begging for deadlock. This is large method, with lots of calls outside ("open calls") including calls that do thread switches ("fireCommunicationsEvent"). Imagine this scenario:
if (msg != null && msg.getLevelOneText().startsWith(NLS.bind(ConnectorServiceResources.MSG_COMM_INVALID_LOGIN, getHostName())))
I think you should not compare NLS messages with startsWith() because this might fail in a BIDI environment. You simply don't have control of what language packs and NLS substitution does to your messages.
should be in a common place rather than ConnectionStatusListener -- what about having it in SimpleSystemMessage? Why wasn't this code needed before the Refactoring? You do have private static SimpleSystemMessage.throwableToDetails() don't you?
Next, in RexecDstoreServer you use MessageFormat.format() -- better use NLS.bind()
While at changing things, constants with CANCELLED should be renamed to CANCELED
Next, the implementation of SystemMessage#clone() is invalid (and has always been invalid). It needs to call super.clone() because the way you do it, if you would clone a SimpleSystemMessage you would come up with a SystemMessage (that's no longer simple). Please read the Javadocs of Object#clone().
Then, in the following classes the Message Strings need to be externalized: this was not possible before your change and thus was marked with //TODO dwd -- but now it should be done: RemoteFileCancelledException, RemoteFileIOException, RemoteFileSecurityException, RemoteFolderNotEmptyException
Messages with "RSE","F","9999" should be migrated to your new SimpleSystemMessage, shouldn't they?
Martin Oberhuber, Senior Member of Technical Staff, Wind River
Target Management Project Lead, DSDP PMC Member
Back to the top