opentripplanner / OpenTripPlanner

An open source multi-modal trip planner
http://www.opentripplanner.org
Other
2.12k stars 1.01k forks source link

Fix copy-on-write in TimetableSnapshot #5941

Open vpaturet opened 5 days ago

vpaturet commented 5 days ago

Summary

As detailed in #5933, there is at least one code path that leads to the concurrent update of Timetable objects while they are being accessed by reader threads. This PR fixes one such code path where a real-time added trip is dissociated from a trip pattern, without using copy-on-write.

Issue

Potentially close #5933

Unit tests

The concurrent access bug itself is not unit-tested. The modified implementation details are tested by the unit tests from the original PR #5726

Documentation

No

codecov[bot] commented 5 days ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 69.45%. Comparing base (15cf5f8) to head (66dedbb). Report is 32 commits behind head on dev-2.x.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev-2.x #5941 +/- ## ============================================= - Coverage 69.46% 69.45% -0.01% + Complexity 17076 17074 -2 ============================================= Files 1937 1937 Lines 73680 73713 +33 Branches 7539 7543 +4 ============================================= + Hits 51182 51200 +18 - Misses 19874 19883 +9 - Partials 2624 2630 +6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

vpaturet commented 1 day ago

This fixes the concurrent access on the Timetable object, but the SortedSet that holds the Timetable is also shared between snapshot: it should also be copied for the fix to be complete. I will rework this.

vpaturet commented 1 day ago

Complete fix: