Code review

[sprunk]

Somebody did a code review of kgb. See #ai for more info.

[19:55] bky: I did a code review of a piece of it as essentially a training exercise. [19:56] bky: One of the things I noticed is a pattern that will make it randomly crash on some JVMs. [19:57] bky: Line 310-315 of MilitaryManager [19:59] bky: join() can experience interrupts from the JVM rather than a user thread. [19:59] bky: This trips a System.exit(-1) to intentionally crash the AI [20:03] Sprung: @Anarchid ^ [20:05] bky: I don't know what the fix should be because I don't know why it's intentionally crashing. [20:06] bky: But it'd involve conditionally retrying the join(). [20:16] Sprung:

boolean ok = false;
while (!ok) {
    try {
        thread.join();
        ok = true;
    } catch (InterruptedException e) {
        // ignore, JVM is just being a dolt
    } catch (Exception e) {
        // something more brutal happened
        ai.printException(e);
        System.exit(-1);
    }
}

[20:16] Sprung: something like this presumably [20:16] bky: yeah [20:17] bky: There's also a separate concurrency issue if the AI experiences 15 frames of lag before the join() succeeds. [20:18] bky: Potentially. [20:18] bky: You might have two simultaneously running invocations of the same method fighting over the same threatmap. [20:19] bky: I can't tell whether that actually happens without knowing whether the Module superclass tries to handle severe lag by not updating on some frames.

[aeonios]

No, the module superclass basically does nothing. It only serves as an interface/template.

I had not heard of KGB producing concurrency issues for anyone, but the thread could easily be removed if it causes problems. I'm not sure if it actually provides any speedup anyway. I'll take a look at it.

[aeonios]

Alright, I incorporated this as is. Seems kosher to me. Not that I've actually had the opportunity to test it, but I really doubt it'll break anything.