-
Notifications
You must be signed in to change notification settings - Fork 522
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
IOLocal
propagation for unsafe access
#3636
Changes from 8 commits
0b88c01
db743e2
716ef32
0a69caf
2775064
270764f
d55489d
2cf72a5
cb3859d
7dce01c
5e171ac
c2f312d
638930d
9174c6a
1987e3a
02a43a6
a7bf748
145fc0e
fa99a5c
6cad03c
bb5d4b1
7517755
8d8e004
3589db4
522677e
6cc4d38
ac88480
49e5c30
925f504
d63a6ff
d4549fb
2502045
535fc8a
d854799
f070552
2cf1d8a
0eec9dd
af84973
1adf368
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1009,7 +1009,7 @@ sealed abstract class IO[+A] private () extends IOPlatform[A] { | |
implicit runtime: unsafe.IORuntime): IOFiber[A @uncheckedVariance] = { | ||
|
||
val fiber = new IOFiber[A]( | ||
Map.empty, | ||
if (IOFiberConstants.dumpLocals) unsafe.IOLocals.getState else Map.empty, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can even go in the opposite direction for It's less clear if/how to do this for fibers started in a |
||
oc => | ||
oc.fold( | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -250,6 +250,10 @@ private final class IOFiber[A]( | |
pushTracingEvent(cur.event) | ||
} | ||
|
||
if (dumpLocals) { | ||
IOLocals.setState(localState) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dumb question: can't we simply do this when we get scheduled on a thread? We know when we're on a thread and we know when we get off of it, so can't we simply set and clear the state respectively at those points? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No we can't, unless we unify how the state is represented. Currently it's a var to an immutable map in the fiber and also in the thread. While the fiber is running its copy of the var may be updated effectually in the runloop so the thread-local copy would need to be kept in sync with that. Or we could drive all updates through the thread-local copy of the var, but then there would be a penalty for accessing it esp. if we are not running on a worker thread. Putting aside technical issues, nobody should be unsafely messing about with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What we can do is set the current fiber in a thread local every time we get scheduled on a thread. Then the unsafe Note this would leave the fiber's |
||
} | ||
armanbilge marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
var error: Throwable = null | ||
val r = | ||
try cur.thunk() | ||
|
@@ -260,6 +264,10 @@ private final class IOFiber[A]( | |
onFatalFailure(t) | ||
} | ||
|
||
if (dumpLocals) { | ||
localState = IOLocals.getAndClearState() | ||
} | ||
|
||
val next = | ||
if (error == null) succeeded(r, 0) | ||
else failed(error, 0) | ||
|
@@ -324,6 +332,10 @@ private final class IOFiber[A]( | |
pushTracingEvent(delay.event) | ||
} | ||
|
||
if (dumpLocals) { | ||
IOLocals.setState(localState) | ||
} | ||
|
||
// this code is inlined in order to avoid two `try` blocks | ||
var error: Throwable = null | ||
val result = | ||
|
@@ -335,6 +347,10 @@ private final class IOFiber[A]( | |
onFatalFailure(t) | ||
} | ||
|
||
if (dumpLocals) { | ||
localState = IOLocals.getAndClearState() | ||
} | ||
|
||
val nextIO = if (error == null) succeeded(result, 0) else failed(error, 0) | ||
runLoop(nextIO, nextCancelation - 1, nextAutoCede) | ||
|
||
|
@@ -391,6 +407,10 @@ private final class IOFiber[A]( | |
pushTracingEvent(delay.event) | ||
} | ||
|
||
if (dumpLocals) { | ||
IOLocals.setState(localState) | ||
} | ||
|
||
// this code is inlined in order to avoid two `try` blocks | ||
val result = | ||
try f(delay.thunk()) | ||
|
@@ -401,6 +421,10 @@ private final class IOFiber[A]( | |
onFatalFailure(t) | ||
} | ||
|
||
if (dumpLocals) { | ||
localState = IOLocals.getAndClearState() | ||
} | ||
|
||
runLoop(result, nextCancelation - 1, nextAutoCede) | ||
|
||
case 3 => | ||
|
@@ -446,6 +470,10 @@ private final class IOFiber[A]( | |
pushTracingEvent(delay.event) | ||
} | ||
|
||
if (dumpLocals) { | ||
IOLocals.setState(localState) | ||
} | ||
|
||
// this code is inlined in order to avoid two `try` blocks | ||
var error: Throwable = null | ||
val result = | ||
|
@@ -460,6 +488,10 @@ private final class IOFiber[A]( | |
onFatalFailure(t) | ||
} | ||
|
||
if (dumpLocals) { | ||
localState = IOLocals.getAndClearState() | ||
} | ||
|
||
val next = | ||
if (error == null) succeeded(Right(result), 0) else succeeded(Left(error), 0) | ||
runLoop(next, nextCancelation - 1, nextAutoCede) | ||
|
@@ -965,6 +997,10 @@ private final class IOFiber[A]( | |
if (ec.isInstanceOf[WorkStealingThreadPool]) { | ||
val wstp = ec.asInstanceOf[WorkStealingThreadPool] | ||
if (wstp.canExecuteBlockingCode()) { | ||
if (dumpLocals) { | ||
IOLocals.setState(localState) | ||
} | ||
|
||
var error: Throwable = null | ||
val r = | ||
try { | ||
|
@@ -976,6 +1012,10 @@ private final class IOFiber[A]( | |
onFatalFailure(t) | ||
} | ||
|
||
if (dumpLocals) { | ||
localState = IOLocals.getAndClearState() | ||
} | ||
|
||
val next = if (error eq null) succeeded(r, 0) else failed(error, 0) | ||
runLoop(next, nextCancelation, nextAutoCede) | ||
} else { | ||
|
@@ -1378,6 +1418,11 @@ private final class IOFiber[A]( | |
var error: Throwable = null | ||
val cur = resumeIO.asInstanceOf[Blocking[Any]] | ||
resumeIO = null | ||
|
||
if (dumpLocals) { | ||
IOLocals.setState(localState) | ||
} | ||
|
||
val r = | ||
try cur.thunk() | ||
catch { | ||
|
@@ -1387,6 +1432,10 @@ private final class IOFiber[A]( | |
onFatalFailure(t) | ||
} | ||
|
||
if (dumpLocals) { | ||
localState = IOLocals.getAndClearState() | ||
} | ||
|
||
if (isStackTracing) { | ||
// Remove the reference to the fiber monitor handle | ||
objectState.pop().asInstanceOf[WeakBag.Handle].deregister() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not good. To avoid this we'll either have to replicate it at both the
cats.effect
andcats.effect.unsafe
levels, or move the thread-localIOLocals
accessors intocats.effect
.