dmcc / PyStanfordDependencies

Python interface for converting Penn Treebank trees to Stanford Dependencies and Universal Depenencies
https://pypi.python.org/pypi/PyStanfordDependencies
68 stars 17 forks source link

jpype fails when using with flask #25

Open staplet3 opened 5 years ago

staplet3 commented 5 years ago

He, I wrapped your library in a flask app and had JPype fail due to an unsafe thread issue. I had to modify the JPypeBackend.py file to attach the thread to the JVM. Changes start on line 45:

num_thread = jpype.isThreadAttachedToJVM()
if num_thread is not 1:
     jpype.attachThreadToJVM()

JPypeBackend.py.zip Attached the modified file here

dmcc commented 5 years ago

Thanks, good to know! Could you send a pull request with the change?

kaushikacharya commented 5 years ago

I have the following observations while using PyStanfordDependencies in flask app, based on the argument passed in run() function of Flask app.

Case 1: threaded=False This runs fine without making any change in PyStanfordDependencies if we also pass the argument _usereloader=False. If use_reloader=True, then it throws error similar to the one mentioned in https://github.com/jpype-project/jpype/issues/279

# Problematic frame:
# C  [_jpype.so+0x59320]  JPJavaEnv::FindClass(char const*)+0x20

One needs to note that in flask\app.py (https://github.com/pallets/flask/blob/master/src/flask/app.py#L977),

  `options.setdefault('use_reloader', self.debug)`

This will set use_reloader=True if debug=True and use_reloader is not explicitly set off.

https://stackoverflow.com/questions/28585033/why-does-a-flask-app-create-two-process This discussion thread in SO mentions effect of use_reloader=False

Case 2: threaded=True https://stackoverflow.com/questions/7861299/jpype-doesnt-work-inside-thread MartinStettner points out the jpype documentation for the issue in Django applications which are multi-threaded. Same solution is mentioned in https://github.com/jpype-project/jpype/issues/81#issuecomment-53989248

For this, as mentioned in https://www.programcreek.com/python/example/107853/jpype.isThreadAttachedToJVM

we would need:

if not jpype.isThreadAttachedToJVM():
            jpype.attachThreadToJVM()

But this will be required not only in https://github.com/dmcc/PyStanfordDependencies/blob/master/StanfordDependencies/JPypeBackend.py#L86 convert_tree(), but also in other functions which uses https://github.com/dmcc/PyStanfordDependencies/blob/master/StanfordDependencies/JPypeBackend.py#L44 self.corenlp = jpype.JPackage('edu').stanford.nlp

Update: https://github.com/jpype-project/jpype/issues/395#issuecomment-502723967 Seems this issue would be resolved in next release of JPype.