gnodet commented on PR #1302: URL: https://github.com/apache/maven/pull/1302#issuecomment-1884946363
> I think this is a good change despite no "speedup" sensed: the original class uses `synchronized` on each listener method, meaning at one time only one thread can enter only one method... and this MAY (most probably is) coggle resolver when uses multiple threads. That was my original intention. However, looking again at what we have here, I'm not sure it fulfils the initial intention. Even if the synchronized keyword has been removed from the main methods, I had to add back some lock because some updates are asynchronous (like initiated, progress, corrupted) but some need the display to be changed synchronously when the last resource transfer is finished (so that the log can continue as normal). Re-reading, I even think there may be cases where the output will be wrong, as the test to check if the updated transfer is the _last_ one looks wrong, as it does not use an up-to-date data. The removal of the entry is done in the display method, which means if two resources are completed very shortly, the test will assume there are still some ongoing resources and the display won't be updated. This needs a bit more work imho. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
