Mark task.code as constant #55125
Mark task.code as constant #55125
Conversation
| jl_value_t *listt = jl_new_struct(jl_uniontype_type, jl_task_type, jl_nothing_type); | ||
| jl_svecset(jl_task_type->types, 0, listt); | ||
|
|
||
| const static uint32_t task_constfields[1] = {0x00000040}; // Set fields 7 as constant |
There was a problem hiding this comment.
@vtjnash should any of the task fields be atomic? I was surprised that things like _state weren't.
There was a problem hiding this comment.
Most likely yes, correctness would seem to demand that (esp for acquire/release of result after the _state read/write). I think it must have gotten missed in my list of data race issues
|
Okay this is currently failing in serialization. |
|
@nanosoldier |
|
Didn't you also have another PR that cleared this field once it started running so that we could GC it a bit earlier? |
| t = Task(()->nothing) | ||
| code = deserialize(s) | ||
| t = Task(code) | ||
| deserialize_cycle(s, t) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
The package evaluation job you requested has completed - possible new issues were detected. |
You are thinking about #48037, but that wasn't part of it. |
sigh |
|
@vtjnash actually I did have #47829, but Jeff thought this was too breaking. @quinnj than used it in ConcurrentUtils.JuliaServices/ConcurrentUtilities.jl@9a26697 |
|
LGTM |
|
Merge? |
@gbaraldi and I chatted about this during JuliaCon.
Changing the code field of a task after it has been scheduled is wrong.
One could technically change it before it has ever been scheduled,
but I see no good reason to allow that (one can just create the Task to point to the right closure in the first place).