Ardesco/driver-binary-downloader-maven-plugin

Mark all goals as thread-safe

ManuelB opened this issue · 5 comments

We are using parallized maven builds (mvn -T1C). We do not experience any issues but we are currently getting the following Warning:

mvn -T1C -X install
...
[WARNING] *****************************************************************
[WARNING] * Your build is requesting parallel execution, but project      *
[WARNING] * contains the following plugin(s) that have goals not marked   *
[WARNING] * as @threadSafe to support parallel building.                  *
[WARNING] * While this /may/ work fine, please look for plugin updates    *
[WARNING] * and/or request plugins be made thread-safe.                   *
[WARNING] * If reporting an issue, report it against the plugin in        *
[WARNING] * question, not against maven-core                              *
[WARNING] *****************************************************************
[WARNING] The following goals are not marked @threadSafe in appstore:
[WARNING] com.lazerycode.selenium:driver-binary-downloader-maven-plugin:1.0.17:selenium

Is it possible to add @threadsafe to the selenium goal?

The plugin is not thread safe. It uses a static download location, and it extracts binaries to a single location, if multiple instances of the plugin try to download/extract at the same time you could end up with corrupted downloads/binaries.

@Ardesco I reviewed the mentioned parts and added synchronized blocks.

ManuelB@833c0ee

We are already running the plugin for multiple month with mvn -T1C and it works without problems.

Is there anything else that I have to do?

To start off with, the fact you have had no problems running with multiple threads over x period of time does not mean that the plugin is thread safe, it just means you haven't got unlucky yet.

Making the code thread safe is rarely as simple as sticking a synchronised keyword in a few places, all you are doing is forcing the code to queue up multiple requests that will be executed in a single thread (In other words you are blocking the code from running in multiple threads).

To make the plugin thread safe we would need to do the following as a minimum:

  • Check all dependencies are thread safe and change them/get their authors to make them threads safe and update them if they are not.
  • Check all static member variables are using thread safe data types and change them if they are not.
  • To ensure acceptable performance levels you should limit the use of synchronized and instead make the code smarter e.g. if multiple threads want to download a file, one thread should download it and then report to the other threads that it was downloaded successfully, you don't want to end up downloading the same file 5 times in sequence (Or in the case of your PR depending upon which call gets in next we may perform 2 downloads, then an unzip, then another download, then two more unzips, etc, etc).
  • The plugin works with external files that are not part of the JVM, we should really be locking them from an os level before working on them (this would also protect you against two maven runs running in parallel in separate terminals).

It is possible to make the plugin thread safe, but it's going to take time, effort and require lots of testing.

Also please do bear in mind that what you are seeing in your maven console is simply a warning, if everything works fine for you, you can just disregard it; it is a warning after all.

@Ardesco thanks a lot for your description. I think I won't get the time from my employer to do the necessary work. For now I will just ignore the warning.