Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Add support for thread safety on async actions only#968

Merged
johnhaley81 merged 1 commit intonodegit:masternodegit/nodegit:masterfrom
srajko:thread-safety-async-onlysrajko/nodegit:thread-safety-async-onlyCopy head branch name to clipboard
Mar 29, 2016
Merged

Add support for thread safety on async actions only#968
johnhaley81 merged 1 commit intonodegit:masternodegit/nodegit:masterfrom
srajko:thread-safety-async-onlysrajko/nodegit:thread-safety-async-onlyCopy head branch name to clipboard

Conversation

@srajko
Copy link
Collaborator

@srajko srajko commented Mar 24, 2016

Adds a middle-ground thread-safety level, which locks async functions but skips locking of sync functions, to avoid the risk of blocking the main thread.

/cc @johnhaley81 @joshaber

@srajko srajko force-pushed the thread-safety-async-only branch from fbd892b to 28ab2f1 Compare March 24, 2016 19:57
@srajko
Copy link
Collaborator Author

srajko commented Mar 24, 2016

It's backwards compatible with NodeGit.enableThreadSafety() but looks like I took away NodeGit.disableThreadSafety() (you would have to use the more general NodeGit.setThreadSafetyStatus).

@srajko srajko force-pushed the thread-safety-async-only branch from 28ab2f1 to 6f08243 Compare March 24, 2016 20:51
@srajko
Copy link
Collaborator Author

srajko commented Mar 24, 2016

BTW the async-only thread-safety is activated using NodeGit.setThreadSafetyStatus(1). NodeGit.enableThreadSafety() still does the full async+sync locking (equivalent to NodeGit.setThreadSafetyStatus(2)).

It might be nice to add more user-friendly identifiers instead of 1 and 2 (and 0 - disabled)

@joshaber joshaber mentioned this pull request Mar 25, 2016
Merged
2 tasks
@joshaber
Copy link
Collaborator

joshaber commented Mar 25, 2016

Thanks @srajko!

It might be nice to add more user-friendly identifiers instead of 1 and 2 (and 0 - disabled)

Definitely agree 👍

@johnhaley81
Copy link
Collaborator

johnhaley81 commented Mar 28, 2016

It might be nice to add more user-friendly identifiers instead of 1 and 2 (and 0 - disabled)

Let's make those strings that live on NodeGit.THREAD_SAFETY to be a little more clear.

@srajko srajko force-pushed the thread-safety-async-only branch from 6f08243 to 1a8d3b2 Compare March 28, 2016 20:24
@srajko srajko force-pushed the thread-safety-async-only branch from 1a8d3b2 to 045ec0e Compare March 28, 2016 20:38
@srajko
Copy link
Collaborator Author

srajko commented Mar 28, 2016

Let's make those strings that live on NodeGit.THREAD_SAFETY to be a little more clear.

👍 added.

@johnhaley81
Copy link
Collaborator

johnhaley81 commented Mar 29, 2016

@srajko thanks!

@johnhaley81 johnhaley81 merged commit efbd8e1 into nodegit:master Mar 29, 2016
@johnhaley81 johnhaley81 deleted the thread-safety-async-only branch March 29, 2016 00:23
joshaber added a commit to atom/atom that referenced this pull request Apr 11, 2016
@joshaber
Update nodegit API usage.
041906c 
This changed in nodegit/nodegit#968.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@srajko @joshaber @johnhaley81
Morty Proxy This is a proxified and sanitized view of the page, visit original site.