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

Add class script wrapper #2033

Merged
merged 4 commits into from May 8, 2023
Merged

Conversation

MaciejG604
Copy link
Contributor

@MaciejG604 MaciejG604 commented Apr 12, 2023

This should avoid deadlocks when threads are run from the static initalizer of wrapper object.

Closes #532 and #1933

The key to this issue is what's visible in the thread dump from this comment

ZScheduler-Worker-1" #15 daemon prio=5 os_prio=0 cpu=109,44ms elapsed=32,71s tid=0x00007f62544c7290 nid=0x22344 in Object.wait()  [0x00007f6214706000]
   java.lang.Thread.State: RUNNABLE
	at issue$minus7301$minusexample$minusko$Photos$$anon$1$$Lambda$481/0x0000000800e2c798.apply(Unknown Source)
	- waiting on the Class initialization monitor for issue$minus7301$minusexample$minusko$
	at zio.ZIO.map$$anonfun$1(ZIO.scala:927)
	at zio.ZIO$$Lambda$87/0x0000000800d3f3e0.apply(Unknown Source)

The background thread worker is waiting for object class to be initialized, but the object class initialization process is waiting for the background thread to finish, this introduces a deadlock.

As stated by David Crosson in #532 encapsulating the wait/join/Await.ready method into a additonal object fixed this in some cases by making the background thread not dependent on the initialization of the outer wrapper object introduced by Scala CLI.

@tgodzik
Copy link
Member

tgodzik commented Apr 12, 2023

Looks like some tests might need fixing up?

@MaciejG604 MaciejG604 force-pushed the wrap-scripts-in-class branch 3 times, most recently from 87fd0c7 to d1f61ca Compare April 17, 2023 13:48
@MaciejG604 MaciejG604 changed the title Change script wrapper to a class Add class script wrapper Apr 19, 2023
@MaciejG604 MaciejG604 force-pushed the wrap-scripts-in-class branch 9 times, most recently from a8a0a61 to 5a8c525 Compare April 25, 2023 08:19
@MaciejG604 MaciejG604 marked this pull request as draft April 26, 2023 14:01
@MaciejG604 MaciejG604 force-pushed the wrap-scripts-in-class branch 2 times, most recently from a69b040 to fcc8b3f Compare April 28, 2023 10:34
@MaciejG604 MaciejG604 marked this pull request as ready for review April 28, 2023 10:34
@MaciejG604 MaciejG604 force-pushed the wrap-scripts-in-class branch 2 times, most recently from 67c4923 to f70d88e Compare April 28, 2023 13:59
Copy link
Contributor

@lwronski lwronski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@Gedochao Gedochao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, commented on some nitpicks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working script-wrapper
Projects
None yet
5 participants