Fix out of bounds in lighting subsystem (#75018)

## About The Pull Request
Fixes #74697

Look at this for loop

https://github.com/tgstation/tgstation/blob/bfba2c59340a5cb2e5fe9eb4705a0c3ff5d6f407/code/controllers/subsystem/lighting.dm#L34-L39

Now look at update corners

https://github.com/tgstation/tgstation/blob/bfba2c59340a5cb2e5fe9eb4705a0c3ff5d6f407/code/modules/lighting/lighting_source.dm#L428-L430

Now look at refresh values. this proc has a chance to delete itself here

https://github.com/tgstation/tgstation/blob/bfba2c59340a5cb2e5fe9eb4705a0c3ff5d6f407/code/modules/lighting/lighting_source.dm#L315-L318
And here

https://github.com/tgstation/tgstation/blob/bfba2c59340a5cb2e5fe9eb4705a0c3ff5d6f407/code/modules/lighting/lighting_source.dm#L331-L334

Now look at the Destroy proc, specifically focus on the needs_update
condition

https://github.com/tgstation/tgstation/blob/bfba2c59340a5cb2e5fe9eb4705a0c3ff5d6f407/code/modules/lighting/lighting_source.dm#L66-L67

We are removing the light from the subsystem source queue while the
subsystem is still iterating through them causing big problems for this
for loop

https://github.com/tgstation/tgstation/blob/bfba2c59340a5cb2e5fe9eb4705a0c3ff5d6f407/code/controllers/subsystem/lighting.dm#L33-L37

which causes the out of bounds exception because loop variable `i` is
not updated accordingly when this list size is reduced mid iteration.

The solution? we have to move the loop variable `i` back whenever the
source is deleted i.e. removed from the list so we don't overflow

## Changelog

🆑
fix: out of bounds when updating lights in lighting subsystem
/🆑

---------

Co-authored-by: LemonInTheDark <58055496+LemonInTheDark@users.noreply.github.com>
This commit is contained in:
SyncIt21
2023-05-24 23:59:56 +05:30
committed by GitHub
parent fd777c6375
commit cd4ed228f2
2 changed files with 58 additions and 41 deletions
+47 -22
View File
@@ -30,59 +30,84 @@ SUBSYSTEM_DEF(lighting)
if(!init_tick_checks)
MC_SPLIT_TICK
var/list/queue = sources_queue
var/list/queue
var/i = 0
for (i in 1 to length(queue))
// UPDATE SOURCE QUEUE
queue = sources_queue
while(i < length(queue)) //we don't use for loop here because i cannot be changed during an iteration
i += 1
var/datum/light_source/L = queue[i]
L.update_corners()
L.needs_update = LIGHTING_NO_UPDATE
if(!QDELETED(L))
L.needs_update = LIGHTING_NO_UPDATE
else
i -= 1 // update_corners() has removed L from the list, move back so we don't overflow or skip the next element
// We unroll TICK_CHECK here so we can clear out the queue to ensure any removals/additions when sleeping don't fuck us
if(init_tick_checks)
CHECK_TICK
else if (MC_TICK_CHECK)
if(!TICK_CHECK)
continue
queue.Cut(1, i + 1)
i = 0
stoplag()
else if(MC_TICK_CHECK)
break
if (i)
queue.Cut(1, i+1)
if(i)
queue.Cut(1, i + 1)
i = 0
if(!init_tick_checks)
MC_SPLIT_TICK
// UPDATE CORNERS QUEUE
queue = corners_queue
for (i in 1 to length(queue))
var/datum/lighting_corner/C = queue[i]
while(i < length(queue)) //we don't use for loop here because i cannot be changed during an iteration
i += 1
var/datum/lighting_corner/C = queue[i]
C.needs_update = FALSE //update_objects() can call qdel if the corner is storing no data
C.update_objects()
// We unroll TICK_CHECK here so we can clear out the queue to ensure any removals/additions when sleeping don't fuck us
if(init_tick_checks)
CHECK_TICK
else if (MC_TICK_CHECK)
if(!TICK_CHECK)
continue
queue.Cut(1, i + 1)
i = 0
stoplag()
else if(MC_TICK_CHECK)
break
if (i)
if(i)
queue.Cut(1, i+1)
i = 0
if(!init_tick_checks)
MC_SPLIT_TICK
// UPDATE OBJECTS QUEUE
queue = objects_queue
for (i in 1 to length(queue))
while(i < length(queue)) //we don't use for loop here because i cannot be changed during an iteration
i += 1
var/datum/lighting_object/O = queue[i]
if (QDELETED(O))
if(QDELETED(O))
continue
O.update()
O.needs_update = FALSE
// We unroll TICK_CHECK here so we can clear out the queue to ensure any removals/additions when sleeping don't fuck us
if(init_tick_checks)
CHECK_TICK
else if (MC_TICK_CHECK)
if(!TICK_CHECK)
continue
queue.Cut(1, i + 1)
i = 0
stoplag()
else if(MC_TICK_CHECK)
break
if (i)
queue.Cut(1, i+1)
if(i)
queue.Cut(1, i + 1)
/datum/controller/subsystem/lighting/Recover()