Hi,
If I understand the code properly, readState() creates RepositoryLocationS from the data read from a DataInputStream and adds those created to the repositories cache.
The way it does this is:
1. Creates the RepositoryLocation
2. Adds it to the cache
3. Sets the label and the repository root if certain conditions are met.
In step 2. some listeners are notified about the new RepositoryLocation (that may also notify other listeners).
As far as I can see, it is possible that some listeners may try to access the label and/or the repository root, and there is no guarantee they will see a consistent state of the RepositoryLocation (regarding the label and the repository root) or to see the real values at all at the time of their access.
So I think it would be safer to change the order of the steps and put adding to the cache as the last step.
The proposed patch is below.
Cheers,
Laca
### Eclipse Workspace Patch 1.0
#P org.tigris.subversion.subclipse.core
Index: src/org/tigris/subversion/subclipse/core/repo/SVNRepositories.java
===================================================================
--- src/org/tigris/subversion/subclipse/core/repo/SVNRepositories.java (revision 4302)
+++ src/org/tigris/subversion/subclipse/core/repo/SVNRepositories.java (working copy)
@@ -305,19 +305,22 @@
int count = dis.readInt();
for(int i = 0; i < count;i++){
ISVNRepositoryLocation root = SVNRepositoryLocation.fromString(dis.readUTF());
- addToRepositoriesCache(root);
- if (version >= REPOSITORIES_STATE_FILE_VERSION_2) {
- String label = dis.readUTF();
- if (!label.equals("")) {
- root.setLabel(label);
- }
- }
- if (version >= REPOSITORIES_STATE_FILE_VERSION_3) {
- String repositoryRoot = dis.readUTF();
- if (!repositoryRoot.equals("")) {
- root.setRepositoryRoot(new SVNUrl(repositoryRoot));
+ try {
+ if (version >= REPOSITORIES_STATE_FILE_VERSION_2) {
+ String label = dis.readUTF();
+ if (!label.equals("")) {
+ root.setLabel(label);
+ }
}
- }
+ if (version >= REPOSITORIES_STATE_FILE_VERSION_3) {
+ String repositoryRoot = dis.readUTF();
+ if (!repositoryRoot.equals("")) {
+ root.setRepositoryRoot(new SVNUrl(repositoryRoot));
+ }
+ }
+ } finally {
+ addToRepositoriesCache(root);
+ }
}
}
------------------------------------------------------
http://subclipse.tigris.org/ds/viewMessage.do?dsForumId=1043&dsMessageId=1191245
To unsubscribe from this discussion, e-mail: [dev-unsubscribe_at_subclipse.tigris.org].
Received on 2009-02-19 13:29:17 CET