Is this a bug?

Hi,

I'm working on a plugin for PyCharm and trying to wrap my head around the threading rules.  In this case I was writing a custom SDK updater and looking at PythonSDKUpdater for reference.  On line  127, executing as a background task it says: "updateSdk(project, null, sdk, PythonSdkType.findSkeletonsPath(sdk))".  It seems to me that PythonSdkType.findSkeletonsPath(sdk) is an unprotected read from a background thread or this this task protected somehow?

4 comments
Comment actions Permalink

As far as I can see, the only place in that code where a race condition is possible is in CompositeProjectRoot.getProjectRoots(), which is accessed from ProjectRootContainerImpl.getRoots(). If someone adds a SDK root exactly at the same time as CompositeProjectRoot.getProjectRoots() is executed, a ConcurrentModificationException may be thrown. Given that SDK root modifications are extremely rare, this doesn't happen in practice.

All the other code in the call chain starting at PythonSdkType.findSkeletonsPath() does not work with any shared mutable state.

0
Comment actions Permalink

Yes, I only see a race for myRoots in ProjectRootContainerImpl. I do not expect this to happen in practice, or at any rate the probability is extremely low. I just wanted to check my understanding of your concurrency management.

Also, I do not want to seem pendantic but a ConcurrentModificationException is not guaranteed to be thrown by HashMap -- it's best effort. I've one been unfortunate enough to witness(in prod) HashMap.get go into an infinite loop during a concurrent modification.

0
Comment actions Permalink

I don't see any HashMap which could cause a CME here. CompositeProjectRoot.myRoots is an ArrayList. ProjectRootContainerImpl.myRoots is a HashMap, but it's initialized in the constructor and not modified anywhere else, so it can't cause a CME.

0
Comment actions Permalink

Yes, I see. The HashMap is never modified after initialization unless 'readExternal' can somehow be called after initialization in the object lifecycle. The only race here is on CompositeProjectRoot 'myRoots' , which is an ArrayList

0

Please sign in to leave a comment.