Ashish, Thanks very much for this bug report! The bug is fixed in commit 41559dcd5a63a2dbf1c280ede5ee26788745fed6, at least it appears so.
Eddie Ashish Sharma wrote: > Hi Eddie, > > I stumbled across another possible bug which I don't completely > understand. I can spell out the symptoms though: > > After updating to the commit of Jun 17, whenever I press Ctrl-C to shut > down my USERLEVEL click script, I get this error message, just before dying: > > click: ../include/click/vector.hh:184: void*& > Vector<void*>::operator[](int): Assertion `i>=0 && i<_n' failed. > Aborted > > Functionality wise, I do not think I noticed any changes when runing my > script, except that this message always pops up. I did a binary search > to narrow down to the exact commit and it is this: > > commit c0c49a09dc8cd78bb8e6e8f9117a18bdb42f54ab > Author: Eddie Kohler <[email protected] <mailto:[email protected]>> > Date: Wed Jun 17 11:38:54 2009 -0700 > > User-level driver: Refactor select handling for multithread. > > which made changes in master.(hh|cc) and routerthread.(hh|cc) to > refactor select handling. > > Hope this helps. > > Ashish > > > > On Wed, Jul 8, 2009 at 7:57 PM, Ashish Sharma <[email protected] > <mailto:[email protected]>> wrote: > > Hi Eddie, > > Thanks for the bug fix. The new patch works fine for me. > > Ashish > > > On Tue, Jun 30, 2009 at 12:30 PM, Eddie Kohler <[email protected] > <mailto:[email protected]>> wrote: > > Hi Ashish, > > Thanks for these patches. I've checked in a somewhat different > version -- let me know if it works for you. > > Eddie > > > > Ashish Sharma wrote: > > Hi Eddie, > > Here is a revised patch that should be thread safe. > Thanks Cliff for pointing this out. > > diff --git a/elements/standard/quicknotequeue.cc > b/elements/standard/quicknotequeue.cc > index 88d8b3a..c590925 100644 > --- a/elements/standard/quicknotequeue.cc > +++ b/elements/standard/quicknotequeue.cc > @@ -46,7 +46,17 @@ QuickNoteQueue::pull(int) > if (h != t) > return pull_success(h, t, nh); > else > + { > + _empty_note.sleep(); > +#if HAVE_MULTITHREAD > + // Work around race condition between push() and pull(). > + // We might have just undone push()'s > Notifier::wake() call. > + // Easiest lock-free solution: check whether we > should wake again! > + if (size()) > + _empty_note.wake(); > +#endif > return 0; > + } > } > > CLICK_ENDDECLS > > > > Thanks. > > Ashish > > On Sun, Jun 28, 2009 at 8:01 PM, Ashish > Sharma<[email protected] > <mailto:[email protected]>> wrote: > > Hi Eddie, > > Thanks for your effort in checking in this new element. > I got a chance > to test it out only now. There is however one bug in > this, the > empty_note.sleep is only called when the queue becomes > empty, but if > the queue is empty to begin with, this goes into a > constant polling > mode, calling pull in a loop. Here is a possible patch > that fixes it. > Please consider applying the following patch. Also the > suggestion of > making the SLEEPINESS_TRIGGER value configurable seems > like a good > idea. I can write a patch if you think it should be done. > > diff --git a/elements/standard/quicknotequeue.cc > b/elements/standard/quicknotequeue.cc > index 88d8b3a..4350346 100644 > --- a/elements/standard/quicknotequeue.cc > +++ b/elements/standard/quicknotequeue.cc > @@ -46,7 +46,10 @@ QuickNoteQueue::pull(int) > if (h != t) > return pull_success(h, t, nh); > else > + { > + _empty_note.sleep(); > return 0; > + } > } > > CLICK_ENDDECLS > > > Thanks > Ashish > > > SUB: Notifier::upstream_empty_signal is always active > > On Sun, Jun 14, 2009 at 9:47 AM, Eddie > Kohler<[email protected] <mailto:[email protected]>> > wrote: > > If you want not even one failed pull(), that > will take some > reorganization of the code to clear the notifier > in a different place (this > would be easy to do). > > > You may be interested in QuickNoteQueue, just > checked in. > > Eddie > > > > Since the valid/invalid test is an expensive > one, what I would like is for the Notifier > signal to act like a test > function, so I only perform the > valid/invalid operation if a queue is > non-empty. Further, in case the queue is not > empty but belongs to the set > of > "invalid" queues, I do not want to put the > packet back at the head of the > queue (of which I cannot think of a simple > way of doing from within the > scheduler element). > > More generally, it seems like you should make > your valid/invalid check > cheaper. There are many possible ways to do > this, including caching old > values of the check and only updating the cache > when necessary. > > Eddie > > > One solution is that I extend the Queue > element to include a test > function > and along with the signal check the test > function. > > Is there a clean way to make the Notifier > signal active only when the > input > queue is really non-empty and not a false alarm? > > Thank you for your time. I would really > appreciate any suggestions or > thoughts. > > Thanks > Ashish > _______________________________________________ > click mailing list > [email protected] > <mailto:[email protected]> > > https://amsterdam.lcs.mit.edu/mailman/listinfo/click > > _______________________________________________ > click mailing list > [email protected] > <mailto:[email protected]> > https://amsterdam.lcs.mit.edu/mailman/listinfo/click > > > _______________________________________________ click mailing list [email protected] https://amsterdam.lcs.mit.edu/mailman/listinfo/click
