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

Bugs in python backend #1004

Open
aaronschif opened this issue May 6, 2020 · 7 comments
Open

Bugs in python backend #1004

aaronschif opened this issue May 6, 2020 · 7 comments

Comments

@aaronschif
Copy link

I am creating an issue rather than a PR because I have not been able to create a working Odb or Refdb backend, but I have found several bugs. Please let me know if I am completely misunderstanding this code.

I believe the issues I cannot get past come from threading in libgit2. I have found that, at times, calling threading.get_ident will cause a segfault. I have tried wrapping all C->python code in PyGILState_Ensure and PyGILState_Release, but this did not work.

I have found the following possible errors:

In my experience, problems like this mean that I am misusing something. Let me know if these appear to be real bugs and I will see if I can at least create test cases to expose them. I don't seem to have a good grasp on the threading segfaults though.

@jdavid
Copy link
Member

jdavid commented May 9, 2020

I think you're right about at least some of the bugs, ping @ddevault since he's the author of this code. For the 3rd one I think the link is not correct, should not be

pygit2_refdb_backend_write(git_refdb_backend *_be,
?

Will give a look at the threading issue.

@ddevault
Copy link
Contributor

ddevault commented May 9, 2020

I think you would be completely bonkers to introduce multithreading into a Python program, let alone a Python program with C modules.

Can you turn your linewise feedback into a patch, for easier commentary and a better path towards merge?

@aaronschif
Copy link
Author

Threads are common enough in python, especially in web. Either way, my python program does not introduce threads (yet, it is web). I am not familiar with libgit2 internals or python c modules but for the reasons I mentioned earlier, my thought is that threading in libgit is not playing well with python.

Thanks for getting back to me. I will clean up my code create a branch and we can continue.

@ddevault
Copy link
Contributor

No, multiprocessing is common in Python, not multithreading. If you invite the Kraken into your program, I'm not going to help you make it comfortable :)

@webknjaz
Copy link
Contributor

@ddevault threading is very common in Python web-frameworks. All of the synchronous frameworks (e.g. CherryPy, Django, Flask) run HTTP handlers (and other things really) in threads, only asynchronous frameworks tend to work with one main thread because of the nature of the async paradigm (IO loop runs in one thread) but they still invite using threads for CPU-bound tasks (because these would introduce long blocking delays blocking the IO loop and making the app freeze).

So no, I'd not say that it's unreasonable to believe that pygit2 would be used in a multithreaded environment. Especially if it's a web-service. In my case, for example, I'm making bots in a form of GitHub Apps, which basically means that I run a web-server where GitHub sends info about the events happening on the platform and since many would be Git-related, guess what, I need something to work with Git and that would be pygit2.

@ddevault
Copy link
Contributor

I maintain that multithreading is supremely stupid and if you want to use it, you'll have to write the patches yourself. I'm not interested in supporting your use-case.

@webknjaz
Copy link
Contributor

I maintain that multithreading is supremely stupid and if you want to use it, you'll have to write the patches yourself. I'm not interested in supporting your use-case.

Relax, I wouldn't ask such a selfishly behaving person to do this anyway. I just wanted to clarify that the case is legit.

jdavid added a commit that referenced this issue Jun 12, 2021
Also fix bug in refresh, issue #1004

And code style changes.
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

No branches or pull requests

4 participants