GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Fix libuv threading issue by johnhaley81 · Pull Request #435 · nodegit/nodegit

Conversation

@johnhaley81

@johnhaley81 johnhaley81 commented Feb 27, 2015

Copy link
Copy Markdown
Collaborator

uv_queue_work is not thread safe

This means that all callbacks are in a highly unstable
state and can bring the system down at any time.

Now we're using an Async handle

That should make out callbacks thread safe and consistent
across platforms again

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
John Haley added 2 commits February 27, 2015 09:53
John Haley
Fix libuv threading issues with callbacks
823b21d
John Haley
Stop skipping the remote transfer test
a8da0a2
@saghul

saghul commented Feb 27, 2015

Copy link
Copy Markdown

Note that uv_async_init is not thread-safe. You should initialize it from the loop thread. Only uv_async_send is thread-safe.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@johnhaley81

johnhaley81 commented Feb 27, 2015

Copy link
Copy Markdown
Collaborator Author

Ahhh interesting. That probably should be a note for that method. I'll fix that in this PR. Thanks for the heads up @saghul!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@maxkorp maxkorp mentioned this pull request Feb 27, 2015
Transfer callback stats are being converted wrong #432
Closed
@johnhaley81

johnhaley81 commented Feb 27, 2015

Copy link
Copy Markdown
Collaborator Author

@saghul I put uv_async_init on the main loop thread but that seems to kill it in linux/osx.

I opened a new PR to show that. Note that Windows succeeds but linux and osx are throwing false positives on the tests.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@johnhaley81 johnhaley81 self-assigned this Feb 27, 2015
@johnhaley81 johnhaley81 added this to the 0.3.0 milestone Feb 27, 2015
@johnhaley81 johnhaley81 added CPP Bug labels Feb 27, 2015
@johnhaley81

johnhaley81 commented Feb 27, 2015

Copy link
Copy Markdown
Collaborator Author

@tjfontaine was saying that we should be doing something like this.

I think I'm going to merge this for right now and tackle that later.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
johnhaley81 added a commit that referenced this pull request Feb 27, 2015
@johnhaley81
Merge pull request #435 from nodegit/fix-libuv-thread
9a859cc 
Fix libuv threading issue
@johnhaley81
johnhaley81 merged commit 9a859cc into master Feb 27, 2015
@johnhaley81
johnhaley81 deleted the fix-libuv-thread branch February 27, 2015 21:36
@tbranyen tbranyen modified the milestone: 0.3.0 Mar 2, 2015
@johnhaley81 johnhaley81 mentioned this pull request Mar 14, 2015
Look into using v8plus_cross_thread_call for spawning callbacks from libgit2 #487
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@johnhaley81 johnhaley81

Labels

Bug CPP

Projects

None yet

Milestone

0.3.0

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@johnhaley81 @saghul @tbranyen
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.