SimReporter is currently a handle to a mutable thing: it holds an Arc<ReporterShared> whose restart history sits behind a Mutex, shared with the SimReporters registry. RFD 726 probably wants simulated reporters to be owned immutable values instead.
The reason for this is that the sim's session history is a tree of immutable states (undo, redo, forking a session from any earlier point). This requires that cloning a state is safe, which it wouldn't be if two states share a mutable SimReporter.
Proposed refactor: dissolve ReporterShared into SimReporter, so the registry owns the reporters and tests borrow them from it rather than holding detached handles. This sort of reaches the end state the existing TODO asks for, though in the opposite direction (the TODO proposes moving the rest of the reporter's state into ReporterShared). The churn at the tests' ereport-creation sites is mechanical.
Also note, the per-reporter collector_id carries a separate TODO which we'll probably resolve in the process somehow... I'm guessing we'll just introduce a constructor that takes the collector id as a parameter.
SimReporteris currently a handle to a mutable thing: it holds anArc<ReporterShared>whose restart history sits behind aMutex, shared with theSimReportersregistry. RFD 726 probably wants simulated reporters to be owned immutable values instead.The reason for this is that the sim's session history is a tree of immutable states (undo, redo, forking a session from any earlier point). This requires that cloning a state is safe, which it wouldn't be if two states share a mutable
SimReporter.Proposed refactor: dissolve
ReporterSharedintoSimReporter, so the registry owns the reporters and tests borrow them from it rather than holding detached handles. This sort of reaches the end state the existing TODO asks for, though in the opposite direction (the TODO proposes moving the rest of the reporter's state intoReporterShared). The churn at the tests' ereport-creation sites is mechanical.Also note, the per-reporter
collector_idcarries a separate TODO which we'll probably resolve in the process somehow... I'm guessing we'll just introduce a constructor that takes the collector id as a parameter.