LattePandaTeam / LattePanda-Development-Support

43 stars 81 forks source link

Concurrency issue with latest modification #7

Open lafrank opened 7 years ago

lafrank commented 7 years ago

Although I think it is a very useful and great work, I also believe - but could be wrong - that there is a serious concurrency issue with latest modification "Update Arduino library for less CPU consumption".

This change has introduced a thread that initiates a timed event that is called in every 30ms to read the serial port, a much better approach than the original code. The problem is however, that the timer will tick and initiate a new call disregarding whether the execution of the previous call has finished or not. This results is multiple, simultaneously running instances of InputProcessor.InputProcess() method.

In my view, InputProcessor.InputProcess() should be enclosed in a Monitor.TryEnter(_serialPort) and Monitor.Exit(_serialPort) try-finally clause to prevent parallel execution.

See pull request #8 for details

LattePanda commented 7 years ago

Laszlo,

Really thanks for your kindly help, that would be very helpful for thousands of lattepanda user.

Is there any other problem or suggestion about the library or lattepanda? any information will be very helpful.

Looking forward to your reply, thanks.

Best Regards, Kelvin Sun LattePanda Team www.lattepanda.comhttp://www.lattepanda.com


From: Laszlo Frank notifications@github.com Sent: Sunday, June 25, 2017 5:13:45 PM To: LattePandaTeam/LattePanda-Development-Support Cc: Subscribed Subject: [LattePandaTeam/LattePanda-Development-Support] Concurrency issue with latest modification (#7)

Although I think it is a very useful and great work, I also believe - but could be wrong - that there is a serious concurrency issue with latest modification "Update Arduino library for less CPU consumption".

This change has introduced a thread that initiates a timed event that is called in every 30ms to read the serial port, a much better approach than the original code. The problem is however, that the timer will tick and initiate a new call disregarding whether the execution of the previous call has finished or not. This results is multiple, simultaneously running instances of InputProcessor.InputProcess() method.

In my view, InputProcessor.InputProcess() should be enclosed in a Monitor.TryEnter(_serialPort) and Monitor.Exit(_serialPort) try-finally clause to prevent parallel execution.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/LattePandaTeam/LattePanda-Development-Support/issues/7, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AQGbkwPZ1W8zlJKXxmFzIRgVLorYs-oaks5sHiTJgaJpZM4OEjrB.

lafrank commented 7 years ago

Hi Kelvin,

Yes, there are a few other modifications what I already pushed to my fork. Only these are not tested so currently I am not creating a pull request.

Kind Regards, Laszlo

On Tue, Jun 27, 2017 at 5:57 AM, LattePanda notifications@github.com wrote:

Laszlo,

Really thanks for your kindly help, that would be very helpful for thousands of lattepanda user.

Is there any other problem or suggestion about the library or lattepanda? any information will be very helpful.

Looking forward to your reply, thanks.

Best Regards, Kelvin Sun LattePanda Team www.lattepanda.comhttp://www.lattepanda.com


From: Laszlo Frank notifications@github.com Sent: Sunday, June 25, 2017 5:13:45 PM To: LattePandaTeam/LattePanda-Development-Support Cc: Subscribed Subject: [LattePandaTeam/LattePanda-Development-Support] Concurrency issue with latest modification (#7)

Although I think it is a very useful and great work, I also believe - but could be wrong - that there is a serious concurrency issue with latest modification "Update Arduino library for less CPU consumption".

This change has introduced a thread that initiates a timed event that is called in every 30ms to read the serial port, a much better approach than the original code. The problem is however, that the timer will tick and initiate a new call disregarding whether the execution of the previous call has finished or not. This results is multiple, simultaneously running instances of InputProcessor.InputProcess() method.

In my view, InputProcessor.InputProcess() should be enclosed in a Monitor.TryEnter(_serialPort) and Monitor.Exit(_serialPort) try-finally clause to prevent parallel execution.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/ LattePandaTeam/LattePanda-Development-Support/issues/7, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ AQGbkwPZ1W8zlJKXxmFzIRgVLorYs-oaks5sHiTJgaJpZM4OEjrB.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/LattePandaTeam/LattePanda-Development-Support/issues/7#issuecomment-311246266, or mute the thread https://github.com/notifications/unsubscribe-auth/AO-XZIN8GfOKFHFNeS5enzSo-ImTWpyJks5sIH2ogaJpZM4OEjrB .