src-d / modelforge

Python library to share machine learning models easily and reliably.
Apache License 2.0
18 stars 13 forks source link

Fix fork hang while import modelforge.backends #7

Closed zurk closed 7 years ago

zurk commented 7 years ago

Here is the same problem occurred as in https://github.com/src-d/ast2vec/pull/50

you can check it with the same example from here https://github.com/bblfsh/python-driver/issues/55

just replace import bblfsh with import modelforge.backends

vmarkovtsev commented 7 years ago

What? Can you prove this? I have never seen that this API started a background thread

zurk commented 7 years ago

Yes, I can. As I said, the same example:

import os
import sys
import time

import modelforge.backends
# And if you import just modelforge it will be fine

def func(n):
    print('Sleep during {} sec...'.format(n))
    time.sleep(n)
    return n

if __name__ == '__main__':
    for n in range(1, 3):
        pid = os.fork()
        if pid == 0:
            # if you do import bblfsh in this place everythink will be fine
            res = func(n)
            print('Sleep in is Done!')
            sys.exit(0)
        else:
            print('os.waitpid is waiting for {}...'.format(pid))
            _, status = os.waitpid(pid, 0)
            print('os.waitpid is fine!')
    print('ok, terminate.')
vmarkovtsev commented 7 years ago

This is awful. Please add the comment which explains why you had to move this import then.

zurk commented 7 years ago

Yes, I don't like it too.

There is a comment in my changes.

# Client should be imported here because grpc starts threads during import
# and if you call fork after that, a child process will be hang during exit

Do you want something more?

codecov[bot] commented 7 years ago

Codecov Report

Merging #7 into develop will decrease coverage by 0.17%. The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop       #7      +/-   ##
===========================================
- Coverage    75.38%   75.21%   -0.18%     
===========================================
  Files           13       13              
  Lines          577      577              
  Branches        93       93              
===========================================
- Hits           435      434       -1     
- Misses         107      108       +1     
  Partials        35       35
Impacted Files Coverage Δ
modelforge/gcs_backend.py 44.71% <0%> (-0.82%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d9fe4b3...6aeae7f. Read the comment docs.

vmarkovtsev commented 7 years ago

Oops, missed it