search

SleepyTrousers / EnderCore

Library mod used by EnderIO, EnderZoo, and others
Creative Commons Zero v1.0 Universal
51 stars 72 forks source link

The singleton Scheduler's task list needs to be thread safe. #60

Closed kylev closed 8 years ago

kylev commented 8 years ago

Since user action events can happen at the same time as scheduled tasks may fire, we get CMEs when one adds to the list while the other removes from it and iterates it.

Minor performance penalty, but not user detectable.

kylev commented 8 years ago

This has been my recent every-few-hours bummer of a crash in my Sky Factory 2.5.4 world.

crash-2016-08-13_16.37.48-client.txt crash-2016-08-13_20.00.35-client.txt crash-2016-08-14_01.11.34-client.txt crash-2016-08-14_01.32.12-client.txt crash-2016-08-14_02.42.11-client.txt

HenryLoenwind commented 8 years ago

That would only help if another thread was accessing the list. But this looks more like the task that is executed adding a new task.

kylev commented 8 years ago

That could be as well. I haven't found that exact pattern in the tracing of Sky Factory (yet). I need to get my dev env for all the parts ironed out.

I think this fix would handle your hypothesis, as well. You think? I've generally designed around any write access to a structure I'm iterating over, but I'm trying to be minimal here.

kylev commented 8 years ago

Also, given this is the only thing that uses the Scheduler right now, I've seen another modder using TimerTask rather than build is own delayed work queue. Maybe that could just get ripped out or replaced?

kylev commented 8 years ago

This is better handled by 9c1936869c452520ea2b8b8cb4c0ebedcf6f2769 and could be back-ported to 1.7.10.