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

jbuf: allocate mutex and lock also jbuf_debug() #693

Merged
merged 9 commits into from
Feb 28, 2023

Conversation

cspiel1
Copy link
Collaborator

@cspiel1 cspiel1 commented Feb 14, 2023

For the RX thread baresip/baresip#2445 jbuf_debug() should also be thread safe.

@cspiel1 cspiel1 added the RX thread RX real-time thread label Feb 14, 2023
@cspiel1 cspiel1 added this to the v2.13.0 milestone Feb 14, 2023
src/jbuf/jbuf.c Outdated Show resolved Hide resolved
src/jbuf/jbuf.c Outdated Show resolved Hide resolved
@cspiel1
Copy link
Collaborator Author

cspiel1 commented Feb 14, 2023

Now I have a better solution. jbuf_flush() should check if the lock is NULL.

@sreimers
Copy link
Member

Now I have a better solution. jbuf_flush() should check if the lock is NULL.

Thats not very reliable. Better fix allocation, so you can rely on mutex. So we handle this on other places too.

@cspiel1
Copy link
Collaborator Author

cspiel1 commented Feb 14, 2023

Now I have a better solution. jbuf_flush() should check if the lock is NULL.

Thats not very reliable. Better fix allocation, so you can rely on mutex. So we handle this on other places too.

Okay, coming soon.

@cspiel1
Copy link
Collaborator Author

cspiel1 commented Feb 14, 2023

How do you like the variant how jbuf_debug() was made thread safe, with short mutex lock phase? Any further ideas?

	struct mbuf *mb = mbuf_alloc(512);
	struct re_printf pfh = {
		vprintf_handler,
		mb
	};

src/jbuf/jbuf.c Outdated
struct re_printf pfh = {
vprintf_handler,
mb
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is better to have straightforward and readable code.

I dont think that you get much performance benefit with this approach.

In general, clean and maintainable code should be preferred over performance.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sreimers once wrote that printing to stdout may block sometimes.

I don't want to print to stdout while the mutex is locked. But I agree also with you and there might be a simpler solution with an mbuf. A commit will follow.

@alfredh alfredh removed this from the v2.13.0 milestone Feb 26, 2023
@sreimers sreimers merged commit c9ff512 into baresip:main Feb 28, 2023
@cspiel1 cspiel1 deleted the jbuf_lock branch February 28, 2023 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RX thread RX real-time thread
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants