Skip to content

Add support for thread safety on async actions only#968

Merged
johnhaley81 merged 1 commit into
nodegit:masterfrom
srajko:thread-safety-async-only
Mar 29, 2016
Merged

Add support for thread safety on async actions only#968
johnhaley81 merged 1 commit into
nodegit:masterfrom
srajko:thread-safety-async-only

Conversation

@srajko

@srajko srajko commented Mar 24, 2016

Copy link
Copy Markdown
Collaborator

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

srajko commented Mar 24, 2016

Copy link
Copy Markdown
Collaborator Author

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

srajko commented Mar 24, 2016

Copy link
Copy Markdown
Collaborator Author

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

joshaber commented Mar 25, 2016

Copy link
Copy Markdown
Collaborator

Thanks @srajko!

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

Definitely agree 👍

@johnhaley81

johnhaley81 commented Mar 28, 2016

Copy link
Copy Markdown
Collaborator

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

srajko commented Mar 28, 2016

Copy link
Copy Markdown
Collaborator Author

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

👍 added.

@johnhaley81

johnhaley81 commented Mar 29, 2016

Copy link
Copy Markdown
Collaborator

@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