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

Launch JVM in new thread #1075

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

Thrameos
Copy link
Contributor

This is an attempt to solve the OSX issue in which the JVM main thread and the GUI thread must be separate or the GUI will go into a deadlock. Unfortunately, I have no way to test this particular PR as I don't have access to that architecture. If someone can tell me if this fixes the deadlock issue (or makes it worse), it would be appreciated.

@codecov
Copy link

codecov bot commented May 30, 2022

Codecov Report

Merging #1075 (6f9dabd) into master (faa41f7) will decrease coverage by 0.03%.
The diff coverage is 75.71%.

@@            Coverage Diff             @@
##           master    #1075      +/-   ##
==========================================
- Coverage   88.63%   88.60%   -0.04%     
==========================================
  Files         112      112              
  Lines       10235    10295      +60     
  Branches     4012     4026      +14     
==========================================
+ Hits         9072     9122      +50     
- Misses        703      712       +9     
- Partials      460      461       +1     
Impacted Files Coverage Δ
native/common/include/jp_context.h 71.79% <ø> (+1.52%) ⬆️
jpype/_core.py 91.30% <23.07%> (-4.09%) ⬇️
native/common/jp_context.cpp 83.52% <87.50%> (+1.90%) ⬆️
jpype/_jproxy.py 98.90% <100.00%> (+0.01%) ⬆️
native/common/jp_methoddispatch.cpp 87.03% <0.00%> (+0.92%) ⬆️
native/common/jp_method.cpp 98.84% <0.00%> (+1.15%) ⬆️

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 faa41f7...6f9dabd. Read the comment docs.

@ap--
Copy link

ap-- commented May 30, 2022

Hi @Thrameos

So I am testing this with the following script with python3.9 on macOS 11.6.5:

import threading
import time
from datetime import datetime

import jpype
from jpype import setupGuiEnvironment, shutdownGuiEnvironment, shutdownJVM, startJVM, JClass

print("JPYPE VERSION", jpype.__version__)


jpype.startJVM()
JFrame = JClass("javax.swing.JFrame")
JLabel = JClass("javax.swing.JLabel")


class CustomGUI:

    def __init__(self):
        self._frame = self._label = None

    def run(self):
        print(f"{type(self).__name__}.run: start")
        self._frame = frame = JFrame("TIME")
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE)
        self._label = label = JLabel("time: ...")
        frame.getContentPane().add(label)
        frame.pack()
        frame.setVisible(True)
        print(f"{type(self).__name__}.run: exit")

    def set_label(self, text):
        if self._label:
            self._label.setText(text)
            return True
        return False


custom_gui = CustomGUI()

def _update_gui_from_python_thread(gui_instance):
    now = datetime.now().isoformat()
    print(f"python: updating from thread: {now}")
    if not gui_instance.set_label(f"time: {now}"):
        print("python: could not update")

t = threading.Timer(1.0, _update_gui_from_python_thread, args=(custom_gui,))
t.start()

print("calling setupGuiEnvironment")
setupGuiEnvironment(custom_gui.run)
# vvvvv the code after setupGuiEnvironment doesn't get called

# here we would want to run python specific things while the app is running...
while True:
    print("hello from main")
    time.sleep(2.0)

shutdownJVM()

Behavior of 1.4.1_dev0 and 1.4.0 seems to be identical. The UI pops up and the label is set but then setupGuiEnvironment seemingly never returns and nothing updates or prints anymore. Closing the UI exits the script without falling into the while loop ('hello from main') in the script.

JPYPE VERSION 1.4.1_dev0
calling setupGuiEnvironment
CustomGUI.run: start
python: updating from thread: 2022-05-30T09:20:06.755945
CustomGUI.run: exit
JPYPE VERSION 1.4.0
calling setupGuiEnvironment
CustomGUI.run: start
python: updating from thread: 2022-05-30T09:23:03.268592
CustomGUI.run: exit

Let me know if I can help by providing any debugging information for you.

Cheers,
Andreas

@Thrameos
Copy link
Contributor Author

Just to confirm. When you say JPype 1.4.1_dev1 you mean this PR? Were you able to modify native/common/jp_context.cc to confirm that the JVM did get a new thread? (At least once in the past I have fooled myself when the site cache used the wrong copy.) Not that I didnt expect there was something else required. The gui hack does two actions. It switches the original JVM/Python to a differ thread, and it lauches an App handler of some kind.

You may be able to kludge the behavior by having matplot lib draw a plot before starting the JVM because they should be launching the app handler. But that is just a guess.

Assuming it did not help, how do suggest replicating the jpype/_gui.py behavior without forcing the JPype to main to a new thread like the gui hack? Thus far we have the JVM on a new thread but we also need to somehow get the App handler on a thread, add a headless option if the gui isnt used, and add testing so that we can confirm operation. I am just not sure how to do that type of modification blind.

@Thrameos
Copy link
Contributor Author

@ap-- I attempted to spawn an AppHelper thread in addition to the Java thread. Perhaps that will resolve the issue. (Still making blind stabs in the dark)

@marscher
Copy link
Member

It'd be really nice, if we have some OSX user test this PR and report back with us.

@Thrameos
Copy link
Contributor Author

Agreed. I tried to get a OSX machine from work to test on, but never had any luck. Thus this one needs to stay open until we get a tester.

@marscher
Copy link
Member

marscher commented May 2, 2023

I guess since for a long time nobody volunteered, we shall postpone it after 1.5?

@ap--
Copy link

ap-- commented May 2, 2023

Hello everyone,

sorry, this dropped off my list a while ago. I'm happy to test and provide feedback and I should be able to find time next weekend.

In any case please don't let this be a blocker for a new release.

Cheers,
Andreas 😃

@marscher
Copy link
Member

marscher commented May 2, 2023

Thanks Andreas, looking forward to the results.

@pelson
Copy link
Contributor

pelson commented Feb 12, 2024

Just to chime in and say that this solves #1169 on Linux. That issue is about starting the JVM on another thread, and then needing to attach as a daemon, so not a big surprise that it solves the issue. Just wanted to report here though 👍

@marscher
Copy link
Member

marscher commented Oct 2, 2024

@pelson So shall we merge it because it fixes #1169 then?

@Thrameos
Copy link
Contributor Author

Thrameos commented Oct 3, 2024

Knowing what I do now the OSX can only be fixed by the jpython method or by delibrately abandoning the main thread so it can serve tge event loop which is a rather poor choice for interactive sessions. But if this solves other issues then I am all for it. Forcing Pythons main thread to be the thread that has to stop the jvm lead to shutdown race conditions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants