Skip to content

Conversation

@daurnimator
Copy link

@daurnimator daurnimator commented Feb 1, 2018

Closes #20 and #23

@daurnimator daurnimator changed the title Save current lua_State for callbacks to use Fix undefined behaviour Feb 1, 2018
@daurnimator
Copy link
Author

Ping?

@karlp
Copy link
Contributor

karlp commented Mar 5, 2018

not forgotten. Just been really busy at work, and because I don't understand lua as well as you, I need to run this through my own tests and try this out before I'm happy with it.

@ironpillow
Copy link

Have similar issues. @karlp can you merge this PR. Really appreciate it!

Lua errors (including allocation failures) would `longjmp` out of the
callbacks, bypassing cleanup in libmosquitto.

This commit changes errors from undefined behaviour (that luckily used
to get reported) to silently discarding errors.

Closes flukso#23
@lePereT
Copy link

lePereT commented May 13, 2021

Please could this be merged. Will allow this library to be used in concurrent programs making use of co-routines

@niecore
Copy link

niecore commented Jun 11, 2021

Any updates on this PR?

@karlp
Copy link
Contributor

karlp commented Jun 11, 2021

For my sake at least, I simply felt uncomfortable with how much this PR had to change. I needed to do a lot more testing on it to understand it, and it has simply never been a priority for me. I've still got this ticket as an "unread email" because it really does appear to be a real issue, but I'm personally unlikely to ever find the time to wrap my head around it. (I'm not a co-routine user, really at all) One of the other maintainers may have more time allocated for this. My complete lack of co-routine usage just make this a hard project.

@icarus75
Copy link
Member

I second @karlp. I also don't have the time to really get to the bottom of this. My major concern is that these proposed changes would cause a regression in existing functionality, which seems warranted with commit statements like:
This commit changes errors from undefined behaviour (that luckily used to get reported) to silently discarding errors.
Having a solid set of unit tests with good code coverage would help. Any takers?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Callbacks occur in same coroutine that mqtt.new is called from

6 participants