Fix possible race in WaitingTaskHolder if reused

[Dr15Jones]

If the same WaitingTaskHolder is reused indirectly by the task it was holding, it was possible to cause the task being held to be lost.

[cmsbuild]

The code-checks are being triggered in jenkins.

[Dr15Jones]

@wddgit here is the fix for the potential problem you spotted.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25803/8238

• This PR adds an extra 12KB to repository

[cmsbuild]

A new Pull Request was created by @Dr15Jones (Chris Jones) for master.

It involves the following packages:

FWCore/Concurrency

@cmsbuild, @smuzaffar, @Dr15Jones can you please review it and eventually sign? Thanks.
@makortel, @Martin-Grunewald, @wddgit this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

[wddgit]

Looks good. I think that would have avoided the problem even without the locks.

[Dr15Jones]

please test

[Dr15Jones]

+1

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/32910/console Started: 2019/01/30 20:50

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

[Dr15Jones]

Looks good. I think that would have avoided the problem even without the locks.
The more I think about it, the less I think that is the case. There was no synchronization mechanism happening within the body of doneWaiting and therefore reusuing the WaitingTaskHolder without an explicit syncrhonization mechanism external to doneWaiting is still a race condition (i.e. there is nothing that forces the CPU cache that holds m_task to get updated across the CPUs).

[wddgit]

After thinking some more, I think you are right. There is no explicit synchronization mechanism.

[cmsbuild]

+1
Tested at: 46118c3
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25803/32910/summary.html

[cmsbuild]

Comparison job queued.

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25803/32910/summary.html

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 0 differences found in the comparisons
• DQMHistoTests: Total files compared: 32
• DQMHistoTests: Total histograms compared: 3097440
• DQMHistoTests: Total failures: 1
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3097242
• DQMHistoTests: Total skipped: 197
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
• Checked 133 log files, 14 edm output root files, 32 DQM output files

[fabiocos]

+1