Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Synchronize the prognostication when recording and replaying the journal #2141

Merged
merged 10 commits into from
Apr 27, 2019

Conversation

pleroy
Copy link
Member

@pleroy pleroy commented Apr 23, 2019

Fix #2107.

Also fix a nasty bug where the methods called by the finalizers (on a separate thread) would not be recorded because the recorder was thread_local.

@@ -57,7 +56,7 @@ class PlayerTest : public ::testing::Test {
TEST_F(PlayerTest, PlayTiny) {
// Do the recording in a separate thread to make sure that it activates using
// a different static variable than the one in the plugin dynamic library.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update the comment, otherwise we are going to be even more confused next time we look at this.

Sorry, something went wrong.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.

Sorry, something went wrong.

@@ -72,6 +72,9 @@ void CDECL principia__ActivateRecorder(bool activate);
extern "C" PRINCIPIA_DLL
void CDECL principia__InitGoogleLogging();

extern "C" PRINCIPIA_DLL
void CDECL principia_VesselMakeSynchronous();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent naming (_ vs. __).

This is really not a VesselFoo function (it does not belong in interface_vessel), it exists only to be called by the journal player; perhaps its name should reflect that (from the point of view of the caller, it is the Player counterpart to ActivateRecorder).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

principia__ActivatePlayer.

@eggrobin eggrobin added the LGTM label Apr 26, 2019
@pleroy pleroy merged commit e95eb6c into mockingbirdnest:master Apr 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Journal replaying is no longer deterministic
2 participants