I think you're right. But before you bother fixing it, test the
newer version of Schedule.java out on CVS. Here it is:
I have no doubt that the new Schedule.java has some bugs but would
like to stomp them. It's got your bug fixed though.
Sean
On Oct 7, 2007, at 12:32 AM, Nicholas Radtke wrote:
> I'm not sure if this is the correct place to post this question,
> but since
> there doesn't seem to be a developers' list, I thought I'd go ahead
> and post
> it here.
>
> I believe I've found a bug in sim.engine.Schedule in Mason 12.
> Specifically,
> there is a execution path that sets but never unsets the variable
> inStep:
>
>> From Schedule.java:
>
> public synchronized boolean step(final SimState state)
> {
> inStep = true;
> final double AFTER_SIMULATION =
> Schedule.AFTER_SIMULATION; // a
> little faster
>
> if (time==AFTER_SIMULATION) return false;
> ...
> }
>
> If time is AFTER_SIMULATION, then inStep, which was set to true at the
> beginning of the method, is never set back to false. This appears
> to be an
> error, as it causes incorrect behavior in the following situation:
>
> for (i = 0; i < SOME_NUM; i++)
> {
> sim.start(); /* Add events to the schedule. */
> while(sim.step());
> sim.finish();
> }
>
> When i = 0, the while loop causes the simulation to run until the
> end of the
> schedule, resulting in time being set to AFTER_SIMULATION and the
> above
> step() code executing, leaving inStep = true. Next, sim.finish() and
> sim.start() (via calls to their parent SimState classes' start()
> and finish()
> methods) result in calls to Schedule's pushToAfterSimulation() and
> reset()
> methods. Both of these methods check inStep, which is currently
> true, and
> set the variable killStep to true as a result. Now, any attempts
> to add new
> events to the just-reset schedule fail because the scheduleOnce()
> method
> checks the variable killStep and does not enqueue new events if
> killStep is
> true. Thus, the for loop will not work as expected the second time
> when i =
> 1 because it was not possible to add any events to the schedule.
>
> The fix seems to be straightforward. Replace
>
> if (time==AFTER_SIMULATION) return false;
>
> with
>
> if (time==AFTER_SIMULATION)
> {
> inStep = false;
> return false;
> }
>
> I've tested this fix and found no side effects... but that doesn't
> mean there
> aren't any.
>
> This problem looks like it has been fixed in the CVS version of
> Schedule.java
> that was posted to this list, but since the new version was
> identified as
> experimental, I thought I'd mention the bug here, in case others
> don't want
> to use the new untested version or the new version is rolled back
> and not
> included in the next stable release.
>
> Nicholas
|