Community
Participate
Working Groups
The implementation of simple artifact repository is not thread safe which will likely cause us great pain as we start doing multi threaded download manager. Note that I don't think we need to be fancy in the solution.
Added "non fancy" synchronization on methods accessing the fields of this class/super classes.
This "non fancy" synchronization does not prevent from getting multiple output streams for the same artifact descriptor. Should the artifact repository care about that?
I think that it depends on how/where the artifact repo stores the bytes until the stream is closed. For example if they are written in a temporary file then it does not really matter since the worst thing that could happen is having the file being downloaded multiple times simultaneously, and once one of the download has been successful then the descriptor is written and all subsequent completed download ends up doing nothing since the descriptor for the artifact is already there.
Stefan are you confident that the solution in place is enough to not cause problems in the 1.0 timeframe?
Need to take a last pass over this as part of 3.5 and our effort on robustness.
Reassigning to John to glance through the code
The synchronization added by Stefan looks good to me. The only holes are getOutputStream and getArtifact(descriptor, stream, monitor), in the case where two threads attempt to read/write the same file at once. Concurrent reads should be ok as long as the callers are passing in different output streams to write to. Concurrent writes will fail - on Windows the second thread will fail to open the file while the first thread is writing, and on Linux the second writer will win and overwrite the first write. I think this is ok. Since we have been doing concurrent downloads in several threads without any problems since before 1.0 I think we are in good shape.
