Skip to content

Conversation

@milseman
Copy link
Contributor

@milseman milseman commented Sep 25, 2020

Adds mocking and tracing infrastructure, test
infrastructure, and trace-based tests.

TODO: Linux testing.

Adds mocking and tracing infrastructure, test
infrastructure, and trace-based tests.
guard 0 == pthread_key_create(&raw, releaseObject) else {
fatalError("Unable to create key")
}
// TODO: All threads are sharing the same object; this is wrong
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lorentey this is where I'm probably using TLS wrong.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, this looks doable -- the key thing is that you only want to create the key here, but leave its value NULL. The setspecific call needs to be moved within withMockingEnabled -- the mocking driver must only be set on threads that want to mock, and the wrappers need to bypass all mocking if they see getspecific return NULL.

Copy link
Member

Choose a reason for hiding this comment

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

Beware of nested withMockingEnabled invocations, it's easy to accidentally overwrite an active pointer & leak memory (setspecific doesn't run the destructor on the previous value -- so saving & restoring will work)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

withMockingEnabled does swap out the state, but I think we can simplify it by having it swap out a reference. Good idea, and we should also add a test for that.

@milseman milseman marked this pull request as draft October 21, 2020 16:27
@milseman
Copy link
Contributor Author

For some reason Github isn't recognizing my latest pushes. Closing and re-opening

@milseman milseman closed this Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants