Bug - Won't Fix Unexpected error while doing rave pickpocket

Winterbay

Active member
Over the last couple of days (can't pinpoint exactly when it started ot happen but the daily debuglogs go back to around 1st of April) my DB meatfarming multi have started generating tons of error reports. Wahting the gcli output it seems to be popping up everytim he fails to pickpocket an item normally and uses the rave pickpocket skill instead. At the end of each such combat an error log is printed. The latest log is attached below.
 

Veracity

Developer
Staff member
You are running an ASH script. Turn on debug logging and try again; the stack trace is meaningless without the debug trace. Also, please attach your script.
 

Winterbay

Active member
There are actually two scripts running I guess. The first is dj_d's farm.ash (for automated adventuring) from the automated ascencion suite and the second one is SmartStasis (for the actual fights) by Zarqon. Which one would you need or do you have both? I've not made any significant changes to any of those so I guess you already have access to both? Should I still upload them?

I'll turn on debug logging tomorrow when I'm not overdrunk.
 

Veracity

Developer
Staff member
If it's happening in-battle, it would be SmartStasis. Please attach it; I don't want to go hunting for it...
 

Winterbay

Active member
Ok, here we go. I ran 5 adventures at the Giant's Castle and in the first four I managed to steal an item with Pickpocket - no error log written, in the fifth the pickpocket attempt failed and SS tried Rave Pickpocket which worked and I got the debugmessage.

Uploaded as a ZIP-file due to the large size of the debuglog (it was 35MB before zipping)
 

Veracity

Developer
Staff member
Wow. Well, that is some peculiar code.

Code:
record dbcnprof { effect e; float p; };
dbcnprof[int] bestdb;
// filter out unavailable combos
boolean can_combo() {
   if (my_class() != $class[disco bandit]) return false;
   int combocost,i;
   bestdb.clear();
   foreach eff in dcomb {
      if (have_effect(eff) > 0 || monster_stat("hp") < combo_damage(eff)+1 || 29 - round < count(dcomb[eff]) ||
          (eff == $effect[none] && stolen != $item[none])) { remove dcomb[eff]; continue; }
      combocost = 0;
      foreach num,sk in dcomb[eff] combocost = combocost + mp_cost(sk);
      if (my_maxmp() < combocost) { remove dcomb[eff]; continue; }
       else if (my_mp() < combocost) continue;
      if (combo_profit(eff,combocost) <= 0) remove dcomb[eff];
       else {
          i = count(bestdb);
          bestdb[i].e = eff;
          bestdb[i].p = combo_profit(eff,combocost);
       }
   }
   if (count(bestdb) > 0) sort bestdb by -value.p;
   return (count(dcomb) > 0);
}

// cast a disco combo
string disco_combo(effect grants, string page) {
   if (!can_combo() || !(dcomb contains grants)) return page;
   int combocost;
   foreach num,sk in dcomb[grants] combocost = combocost + mp_cost(sk);
   if (my_mp() < combocost) return page;
   foreach num,sk in dcomb[grants] page = try_skill(page,sk);
   return page;
}
// casts combos in
string best_combos(string page) {
   if (can_combo() && count(bestdb) > 0) foreach num,rec in bestdb
     page = disco_combo(rec.e,page);
   return page;
}

It is crashing in best_combos, trying to assign "rec" in the foreach loop.

Look at how that works:

can_combo creates and fills in the map bestdb, which is a sorted map of possible disco combos.
possible disco combos are in dcomb - and can_combo removes combos from the map, if they are too expensive - or if the combo is "steal" and you just stole...
This is an expensive function.

Note that it clears and recreates "bestdb".

best_combos calls can_combo and then iterates over the resulting map, calling disco_combo for everything in it.
disco_combo itself calls can_combo - which clears and regenerates that map.

So, best_combos is iterating over a map which is changing out from under it. It looks at index 0, 1, ... and so on - based on how many entries were in the map when it called can_combo itself - but by the time it wants to look at index 2, the map has been changed to no longer have that many entries in it.

I'd have to call that a coding error in the ASH program, but ASH should not throw a debug log in this case, and I will consider what needs to be done to fix that...
 

Veracity

Developer
Staff member
For reference, this ASH script reproduces the bug:

Code:
int [int] map;

map[0] = 10;
map[1] = 20;

foreach key, val in map
{
  remove map[1];
}
 

Veracity

Developer
Staff member
Interesting. ASH maps are coded using Java TreeMaps. I've modified "foreach" to iterate over the keys using a Java Iterator - and the above program now throws a Java ConcurrentModificationException.

This exception may be thrown by methods that have detected concurrent modification of an object when such modification is not permissible.

For example, it is not generally permssible for one thread to modify a Collection while another thread is iterating over it. In general, the results of the iteration are undefined under these circumstances. Some Iterator implementations (including those of all the collection implementations provided by the JRE) may choose to throw this exception if this behavior is detected. Iterators that do this are known as fail-fast iterators, as they fail quickly and cleanly, rather that risking arbitrary, non-deterministic behavior at an undetermined time in the future.

Note that this exception does not always indicate that an object has been concurrently modified by a different thread. If a single thread issues a sequence of method invocations that violates the contract of an object, the object may throw this exception. For example, if a thread modifies a collection directly while it is iterating over the collection with a fail-fast iterator, the iterator will thow this exception.

Indeed. the results of the iteration are undefined if you are modifying the map within foreach. I'm inclined to pass this runtime error right on up to the user.
 

Veracity

Developer
Staff member
Revision 8382 throws an ASH runtime error in the face of this buggy behavior, rather than putting an "Unexpected Error" backtrace into the debug log. You'll still get an error, unfortunately, until SmartStasis is fixed...
 

zarqon

Well-known member
Ew. Okay, that needs to be redone. Thanks for debugging that!

I haven't actually been a DB since I added the admittedly scrodged-together support for combos to SS...
 
Last edited:

jasonharper

Developer
r8386 restores the ability to use 'remove' from within a foreach loop, as long as you're removing the current key (or will be immediately breaking out of the loop).
 

Bale

Minion
I guess I didn't need to be so quick to fix Universal recovery to deal with that. Heh.

Thanks jason.
 
Top