search

skypacer210 / vim

Automatically exported from code.google.com/p/vim
0 stars 0 forks source link

Python thread are not running in background #103

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Hi,

While trying to fix an issue on clang_complete plugin 
(https://github.com/Rip-Rip/clang_complete/issues/216), it appears that 
background threads created by the plugin are only running while vim execute 
python code. Which means that those threads do not execute while vim is idle.

In 2004 Hari Krishna Dara had the same issue, which seemed to be resolved. The 
original thread with the test-case: 
http://marc.info/?l=vim&m=110029576819749&w=2

If you try to run that test, you'll also notice that ":q" is not working, 
you'll need to kill vim!

Vim version:
vim 7.3.754 on ArchLinux x86_64 (using stock binary).

Thanks!
Xavier

Original issue reported on code.google.com by degu...@gmail.com on 16 Jan 2013 at 4:12

GoogleCodeExporter commented 9 years ago 
Wondering whether it depends on the kind of code that is executed in the 
thread. I did successfully launch threads that are only sleeping and then 
incrementing variable in background on amd64 (and unsuccessfully on arm with 
the same code, observing the same behavior your report here).

For this particular problem (and almost any of this kind) there is a workaround 
though: just use multiprocessing module (+ don’t forget about disabling all 
vim signal handlers in spawned process and terminating the process on VimLeave 
event in parent process).

Original comment by zyx....@gmail.com on 16 Jan 2013 at 4:40

GoogleCodeExporter commented 9 years ago 
Similar to the one you linked in http://marc.info/?l=vim&m=110029576819749&w=2. 
And it in fact runs fine here (vim-7.3.762, amd64).

Original comment by zyx....@gmail.com on 16 Jan 2013 at 4:44

GoogleCodeExporter commented 9 years ago 
Spawning process instead of thread is not an option in my case…

I just tried with a vim-7.3.762 and can still reproduce 100% of the time. I 
should have mentionned that this is with python2, and python3 is not compiled 
in.

Original comment by degu...@gmail.com on 16 Jan 2013 at 4:50

GoogleCodeExporter commented 9 years ago 
I also use Python2 for the test, but I use both pythoninterp=dynamic and 
python3interp=dynamic on amd64 and AFAIR non-dynamic python2-only builds on 
arm. Maybe this matters.

Why this is not an option? If caches that are talked about are per-process ones 
you can just move all libclang-related code to the separate process; you are 
not likely to have performance impact notable by user. And processes are 
forcefully terminatable, so “shutdown of vim takes several seconds” or 
“unable to quit vim when thread is running” issues should not appear.

Original comment by zyx....@gmail.com on 16 Jan 2013 at 5:09

GoogleCodeExporter commented 9 years ago 
Yes you're right about multithreading. I'm just afraid of the cost of sending 
completion results from the "completion process".

But anyway, this is not related to this bug :).

On which platform are you testing? I would guess Windows since 
pythoninterp=dynamic seems to only be available on this platform.

Original comment by degu...@gmail.com on 16 Jan 2013 at 5:37

GoogleCodeExporter commented 9 years ago 
I just tried manually disabling PY_CAN_RECURSE from if_python.c, and the thread 
seems to be running in background. In clang_complete the correct behaviour can 
also be seen.

Original comment by degu...@gmail.com on 16 Jan 2013 at 5:53

GoogleCodeExporter commented 9 years ago 
I did find the culprit!

Reverting commit 7f10daa706bb solves the issue (the test-case is correclty 
incrementing the variable in background). It also prevents vim to compile when 
PY_CAN_RECURSE is not set.

It doesn't solve the problem of non-terminating threads, but that's also 
another issue.

Original comment by degu...@gmail.com on 16 Jan 2013 at 6:16

GoogleCodeExporter commented 9 years ago 
> On which platform are you testing? I would guess Windows since 
pythoninterp=dynamic seems to only be available on this platform.

Nope. It is available anywhere, but pythoninterp=dynamic is the only choice if 
you want to have both python and python3 support in one binary. It is Gentoo on 
amd64. And Ubuntu ARM (efika MX smartbook modification of Ubuntu, stripped of 
almost any package that was there in a fresh installation).

> Reverting commit 7f10daa706bb solves the issue (the test-case is correclty 
incrementing the variable in background). It also prevents vim to compile when 
PY_CAN_RECURSE is not set.

Now when you mentioned it I remember I saw some (threading?) problems with this 
commit on vim-dev. Did not came to my mind as I forgot about threads as they 
are not working on ARM and are not forcefully terminatable. This is also why I 
suggest processes to anyone wanting to use threads.

Though reverting commit is not an option: it itself is solving some issues. 
Better to check why I have the code running correctly and you do not.

Original comment by zyx....@gmail.com on 16 Jan 2013 at 2:59

GoogleCodeExporter commented 9 years ago 
Maybe the bug depend on python's version? Mine is 2.7.3.

And yes I know that reverting the culprit commit is not really an option: it 
does fix a bug. However, the fix may not be the correct one. Is there a 
test-case for that bug to see if we can fix it in another way?

Original comment by degu...@gmail.com on 17 Jan 2013 at 5:58

GoogleCodeExporter commented 9 years ago 
Hum, I'm wondering whether the GIL is not taken too much time.

If we read the description of PyEval_InitThreads(), it says "it is guaranteed 
that the lock has been created and that the calling thread has acquired it". 
Calling PyGILState_Ensure() also takes the GIL. Which means that at init time, 
the GIL is taken 2 times, but only released once in if_python.c:767 (by calling 
Python_SaveThread()). Since the GIL is never released, that would explain why 
the background threads are not allowed to run.

Original comment by degu...@gmail.com on 17 Jan 2013 at 7:20

GoogleCodeExporter commented 9 years ago 
Changing the initial value of pygilstate to PyGILState_LOCKED may be the 
solution? (I'm only supposing since I don't know how the initial bug could be 
triggered)

Original comment by degu...@gmail.com on 17 Jan 2013 at 7:23

GoogleCodeExporter commented 9 years ago 
I also noticed the problem when using clang_complete. Last time closing Vim 
took 2 hours to complete! I'm using Vim 7.3.762 under Cygwin with Python 2.6.8.

Original comment by lech.lor...@gmail.com on 17 Jan 2013 at 11:24

GoogleCodeExporter commented 9 years ago 
Here is how I patched my source.

Original comment by jonathan...@gmail.com on 26 Jan 2013 at 1:25

Attachments:

GoogleCodeExporter commented 9 years ago 
That patch seems to fix the issue indeed.

Original comment by degu...@gmail.com on 30 Jan 2013 at 3:45

GoogleCodeExporter commented 9 years ago 
The patch is included as 7.3.786.
Please reopen this issue if it didn't fix the problem.

Original comment by brammool...@gmail.com on 30 Jan 2013 at 10:42