Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EventQueue: impossible to cancel event with negative unique id #11778

Closed
furavikt opened this issue Oct 30, 2019 · 5 comments · Fixed by #11782
Closed

EventQueue: impossible to cancel event with negative unique id #11778

furavikt opened this issue Oct 30, 2019 · 5 comments · Fixed by #11782

Comments

@furavikt
Copy link

Description of defect

Function 'call_every' from EventQueue class should return unique ID that represents the posted event or an ID of 0 if there is not enough memory. If ID is greater than 0, everything is perfect. Sometimes it returns negative value (different) and such event is not working properly, ie event is running well but function 'time_left' returns invalid value -1 and it is not possible to cancel this event (it's running forever).

Target(s) affected by this defect ?

NUCLEO-F439ZI

Toolchain(s) (name and version) displaying this defect ?

What version of Mbed-os are you using (tag or sha) ?

mbed-os-5.13.4 and mbed-os-5.10.0

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

How is this defect reproduced ?

Run this code. If is everything perfect, it will prints everytimes 10 numbers per a line to console. Everytimes the event with negative ID occurs, the amount of number increases by 10.

Serial      pc(SERIAL_TX, SERIAL_RX); // tx, rx
Thread      t;
EventQueue  _queue;
int16_t     id, cntr = 0;

void dummy_function(){
    pc.printf("%02d ", cntr);
}

void main(){
    pc.printf("Dispatch event queue from threads context\n");
    t.start(callback(&_queue, &EventQueue::dispatch_forever));

    while(1){
        // Create new event
        id = _queue.call_every(100, callback(&dummy_function));
        if(id == 0){
            pc.printf("ERROR\n\n");
        } else {
            pc.printf("\nCreated periodic event with ID %d (time_left %d)\n", id, _queue.time_left(id));
        }
        ThisThread::sleep_for(1000);

        // Cancel event
        pc.printf("\nCancel event with ID %d (time_left %d)\n", id, _queue.time_left(id));
        _queue.cancel(id);
        ThisThread::sleep_for(100);

        cntr++;
    }
}
@ciarmcom
Copy link
Member

Internal Jira reference: https://jira.arm.com/browse/MBOTRIAGE-2299

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 31, 2019

cc @ARMmbed/mbed-os-core

@kjbracey
Copy link
Contributor

I've looked at this, and I can see potential flaws, but I'm not sure how you're getting a negative ID in your example.

The ID you're given is formed from an 8-bit internal ID shifted up above a buffer offset, which is sized according to the size of the buffer.

For the combined result to be negative, the event queue buffer size would have to be 24-bit (so >= 8MiB), as far as I can see.

I'll post a patch making some changes, and we can see if it helps anything. But my test requires a big buffer...?

@kjbracey
Copy link
Contributor

kjbracey commented Oct 31, 2019

Ah, running your specific example, I note that you're putting the ID into a int16_t. This issue is that that assignment is overflowing. With the variable being int there should be no problem (until the queue size is >=8MiB).

@furavikt
Copy link
Author

furavikt commented Nov 1, 2019

@kjbracey-arm Good point, the ID in example above is only 16 bit number and that's the problem. In my real code there was also a 16 bit temporal variable for debug and it overflows... I apologize for the stupid mistake. The EventQueue is working properly until the queue size is >= 8MiB. Thank you for your time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants