billhsu / jUART

Cross platform browser plugin for serial port communication from JavaScript
202 stars 76 forks source link

sendmulti() dangerously shares resources with send() #35

Open gmkarl opened 9 years ago

gmkarl commented 9 years ago

Inspecting the send code paths, I see that sendmulti() and send() share the same message buffer(send_msg), but both treat the buffer as if they are solely in control of it.

send_multi_complete() empties send_msg when it is done. Meanwhile, do_send() checks if send_msg is empty to determine if its asynchronous loop is running! Similarly, it queues its individual bytes on send_msg which will be emptied if a sendmulti() is also running.

It looks like sendmulti() may also have a bug where data is dropped if it is queued up but not ready to be sent yet. send_multi_complete() clears send_msg when done, without calling send_multi_start() again, and do_multi_send() does not call send_multi_start() if there is already data being sent in send_msg; it simply adds the new data to the end.

A good solution to these issues might be to merge shared behavior between the two sets of functions into one set of functions. Maybe a different way of keeping track of data in transit, such as a second queue or a flag.

gmkarl commented 9 years ago

I can't seem to reproduce the asynchronous behavior allowed for in the source. I haven't yet been able to get send_msg to fill up with more than just one item. I can imagine a serial driver blocking to send a byte and these issues happening, but that's hard to replicate without coding the behavior up specifically to test.

bvbfan commented 8 years ago

Are you sure your async api really works? About me, only when send_msg.empty() == true, if you have pending bytes in queue you don't call send_xx (no matter multi or not) when last sending end queue is cleared in send_xx_complete and you miss next one, no?

bvbfan commented 8 years ago

At last but not least, serial protocol is completely sync i.e. you can't send when you have pending reading.