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

Made all accesses to PFSQLiteDatabaseResult & Statement thread-safe. #511

Merged

Conversation

richardjrossiii
Copy link
Contributor

I think this is what was causing the crashes seen in #290, but I'm not 100% sure.

At the very least, accessing them this way was in violation of the SQLite spec.

#import "PFThreadsafety.h"

// PFThreadsafetySafeDispatchSync, but with a return type.
#define thread_safe_sync(queue, block) ({ \
Copy link
Contributor

Choose a reason for hiding this comment

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

Any chance we can make this a function?
This is not going to be nice (probably), but might feel better than having 2 defines that do the same thing...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how we can, seeing as the point of this macro is to be able to easily return any type, not just id or int.

We could stick it in PFThreadsafety.h, perhaps?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good, let's do it! :)

@nlutsenko
Copy link
Contributor

Few nits, otherwise lgtm.
Love the idea!

@richardjrossiii richardjrossiii force-pushed the richardross.localdatastore.sqlite.threadsafe branch from 484a572 to 66f9bca Compare November 6, 2015 22:25
@richardjrossiii
Copy link
Contributor Author

PR updated.

@nlutsenko
Copy link
Contributor

Awesome! Please ship!

I think this is what was causing the crashes seen in #290, but I'm not 100% sure.

At the very least, accessing them this way was in violation of the SQLite spec.
@richardjrossiii richardjrossiii force-pushed the richardross.localdatastore.sqlite.threadsafe branch from 66f9bca to 9757503 Compare November 10, 2015 21:08
richardjrossiii added a commit that referenced this pull request Nov 10, 2015
….sqlite.threadsafe

Made all accesses to PFSQLiteDatabaseResult & Statement thread-safe.
@richardjrossiii richardjrossiii merged commit 5863a69 into master Nov 10, 2015
@richardjrossiii richardjrossiii deleted the richardross.localdatastore.sqlite.threadsafe branch November 10, 2015 21:32
@coolnalu
Copy link

Awesome! It's HUUUUUUUUGE!

@richardjrossiii
Copy link
Contributor Author

@coolnalu No problem <3

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.

4 participants