Skip to content

Commit

Permalink
doneSleeping
Browse files Browse the repository at this point in the history
  • Loading branch information
@durban
durban committed Apr 23, 2023
1 parent 8bc1940 commit 3da03b9
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions core/jvm/src/main/scala/cats/effect/unsafe/WorkerThread.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,9 +369,12 @@ private final class WorkerThread(
// the only way we can be interrupted here is if it happened *externally* (probably sbt)
if (isInterrupted())
pool.shutdown()
else if (polled || !parked.get()) // Spurious wakeup check.
else if (polled || !parked.get()) { // Spurious wakeup check.
if (parked.getAndSet(false)) {
pool.doneSleeping()
}
return polled
else // loop
} else // loop
()
}
false
Expand Down

5 comments on commit 3da03b9

@armanbilge
Copy link
Member

Choose a reason for hiding this comment

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

@durban should we do parked.getAndSet(false) unconditionally? Or should we only do that if polled == true?

@durban
Copy link
Contributor Author

@durban durban commented on 3da03b9 Apr 24, 2023

Choose a reason for hiding this comment

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

Probably only if polled == true. It's just unnecessary (but semantically harmless) work otherwise, I think.

@armanbilge
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, one more thing about this: can you give any pointers about how to write a test that exercises the bug this patches? I tried last night and failed miserably, I'm going to try again tonight. Thanks!

@durban
Copy link
Contributor Author

@durban durban commented on 3da03b9 Apr 24, 2023

Choose a reason for hiding this comment

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

I'm not really sure, I always tested with the FS2 test (which practically always failed on my machine). I guess the worker threads should go into untimed parking frequently (parkLoop), which must be finished by a polling system event, so that it will take the problematic branch. How to do that... I'm not sure. If the test does a lot of blocking, that could cause the compute threads to park... how to do the event, I don't know.

@armanbilge
Copy link
Member

Choose a reason for hiding this comment

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

Thanks, finally got a test in place! Merged into my PR :)

Please sign in to comment.