Fix race condition in DAQ modules for 80X (make new PR) #12277
Conversation
…g, with other thread already requests next event from source. Source can then open next LS (internally) and report event number in past LS to the FastMonitoringService. In this case it is possible to run preEndLumi triggered by exception later than source report, in which case exception check was (incorrectly) being skipped.
compilation fixes Conflicts: EventFilter/Utilities/interface/FedRawDataInputSource.h
|
A new Pull Request was created by @smorovic (Srecko Morovic) for CMSSW_8_0_X. Fix race condition in DAQ modules for 80X (make new PR) It involves the following packages: EventFilter/Utilities @mommsen, @cvuosalo, @cmsbuild, @emeschi, @slava77 can you please review it and eventually sign? Thanks. |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
-1 Tested at: ea4c1ce you can see the results of the tests here: The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
|
|
||
| } | ||
|
|
||
| auto sourceReport = inputSource_->getEventReport(lumi, true); |
There was a problem hiding this comment.
It's not clear to me that inputSource_ is guaranteed to be non-null at this point. It would be safer to put in a check before dereferencing this pointer.
|
Jenkins tests are OK. The unit test errors don't seem related to this PR. Comparison differences are due to inclusion in the test of #12085, which adds the full5x5 shower shapes to non-GED photons. |
|
@cmsbuild please test |
cmsbuild
added
tests-pending
comparison-pending
and removed
comparison-available
tests-rejected
labels
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
+1 Fixing rare multi-threading race condition in event processing by DAQ modules. There should be no change in monitored quantities. #12200 and #12201 are the 75X and 76X versions of this PR, and they have already been approved by Reco. The code changes are satisfactory, and Jenkins tests against baseline CMSSW_8_0_X_2015-11-09-1100 show no significant differences, as expected. |
|
+1 |
cmsbuild
added
daq-approved
fully-signed
and removed
daq-pending
pending-signatures
labels
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
|
+1 |
Fix race condition in DAQ modules for 80X (make new PR)
Same changes as in #12202 with conflicts resolved.