bitburner-official / bitburner-src

Bitburner source code.
Other
839 stars 273 forks source link

BUGFIX: add stopFlag check before running main #1689

Closed Caldwell-74 closed 1 month ago

Caldwell-74 commented 1 month ago

There are two edge cases where the stopFlag of a script can be set inbetween ns.exec/ns.run and the main function of that new script beeing called

1: when the script needs to compile and the player kills the script before that is done 2: when an atExit callback starts a new script instance while the game is cleaning up in the prestige code

test script (make sure to the reload the page inbetween runs to guarantee a not compiled "noNS.js")

export async function main(ns) {
  // case 1 
  ns.write("noNS.js", noNS, "w")
  const pid = ns.run("noNS.js")
  ns.kill(pid)
  await ns.sleep(0)
  // case 2
  ns.atExit(() => ns.run("noNS.js"))
  ns.singularity.softReset()
}

const noNS = `
export function main(ns) {
  console.log("i ran")
  try {
    ns.tail()
  } catch (e) {
    console.log(e)
  }
}`

i truly believe that when the stopFlag is already set we shouldnt even start the main function yes that could break player scripts that rely on these edge cases but they can still get a "dead script running" without these edgecases

edit.: i should add that i tested the change quickly on the current web dev version and my modified version on the current web dev version it wrote to the console and on the modified version it didnt

d0sboots commented 1 month ago

It's pretty tricky to actually observe this; I don't think this is truly a "bugfix" in that I don't think it's wrong for a script to execute until the first ns call (similar to how "kill" will only actually kill the script once it gets to the next ns call). But there's also no problems with doing this.

d0sboots commented 1 month ago

Oh, I should add that you can (could) observe this even when the script is already compiled; the asynchronous nature of script launching is enough to give control back to the main function and let it kill the child before it starts.

Caldwell-74 commented 1 month ago

right maybe MISC would have been better

and yeah i realized after that the already compiled case also can trigger this but its kinda covered by case 2 :p