Skip to content

[src] Improving how shell commands are called from python#1595

Merged
danpovey merged 2 commits intokaldi-asr:kaldi_52from
danpovey:kaldi_52_python_changes
Apr 30, 2017
Merged

[src] Improving how shell commands are called from python#1595
danpovey merged 2 commits intokaldi-asr:kaldi_52from
danpovey:kaldi_52_python_changes

Conversation

@danpovey
Copy link
Contributor

No description provided.

@danpovey
Copy link
Contributor Author

@vimalmanohar and @vijayaditya, you may want to look at these changes just for your interest (and in case you see any problems- but not important, as I'm testing it).

@vimalmanohar, this is the way I was suggesting to handle the background processes when we spoke about this before-- i.e. using python threads rather than the polling-based method.

@danpovey danpovey force-pushed the kaldi_52_python_changes branch 2 times, most recently from f6493f8 to 4bd81ce Compare April 29, 2017 20:48
@danpovey danpovey force-pushed the kaldi_52_python_changes branch from 4bd81ce to 150af1c Compare April 29, 2017 23:06
@danpovey danpovey merged commit 6a4c50e into kaldi-asr:kaldi_52 Apr 30, 2017
@danpovey
Copy link
Contributor Author

danpovey commented May 4, 2017

@vimalmanohar and @vijayaditya,
actually there does seem to be a slight problem with this way of doing things.
The script occasionally leaves child processes un-reaped, in the zombie state, and if the main thread was waiting on those children, this can delay the execution of the program. I've seen it but it can't be reliably reproduced. It never waits forever. I'm guessing that either it's waiting for some other process to finish (one which is in a background thread and which we "shouldn't" be waiting on), or python's threading implementation for waiting for things is based on some kind of polling interval that can get large.

My best guess is that this is due to some kind of deficiency in the way Python implements threads.
I haven't decided what to do. One possibility is instead of waiting using p.communicate(), to have the threads manually poll somehow. This may somehow bypass whatever bug in python itself is being activated.

Of course, you may point out some problem with my reasoning.

@danpovey
Copy link
Contributor Author

danpovey commented May 4, 2017

From reading on the internet and searching, it seems that there may be some kind of no-no about forking from a threaded process, which we may be violating.
e.g https://rachelbythebay.com/w/2011/06/07/forked/
I don't fully understand it- apparently the 'logging' module may be a problem here because it uses mutexes. Something about the mutexes being duplicated in the child process of fork(). Maybe something bad happens if someone calls a logging function at the same time as a background thread is launching a job.

I may try to change the code so that it creates the child process from the main thread, and just waits for it from the background thread, and I'll see if the problem goes away.

@danpovey
Copy link
Contributor Author

danpovey commented May 4, 2017

The fix seems to have worked.

Skaiste pushed a commit to Skaiste/idlak that referenced this pull request Sep 26, 2018
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

Successfully merging this pull request may close these issues.

1 participant