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

ofThreadChannel::clear() to clear the channel #7921

Merged
merged 2 commits into from May 15, 2024

Conversation

artificiel
Copy link
Contributor

title says it all; proves useful if unconsumed data in the channel is known to be stale

/// Clears the queue (useful if only the latest
/// data is meant to be transferred (i.e. no queue))
void clear() {
if (!queue.empty) {
Copy link
Contributor

Choose a reason for hiding this comment

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

.empty()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah wow!

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@NickHardeman
Copy link
Contributor

@artificiel a clear method would be very helpful!
Does it need a condition.notify_all(); if it is cleared?

What are your thoughts about a open() function, since the channel can be closed but not re-opened?

@artificiel
Copy link
Contributor Author

condition.notify_all() = good question; i would assume the condition is to trigger a receive? in this case we're not flushing but abandoning the queue. but maybe, I'm not 100% sure...

I've been using it under fair load (12 instances pushing a total of 1000 msg/sec, and when the consuming thread is a bit busy, like when a gui window drag hiccup freezes the framerate for a fraction of a second, an irrecoverable backlog occurs. with the clear I haven't got crashes, and never "old" data.

for the open/close = also good question; I haven't been in a need of closing a channel — I guess that if there is a higher logic that connects/disconnects things, the access logic to the thread channel would be controlled there, more than within the thread channel itself?

so the question would be: why close the thread channel? (i.e. why use an internal state for a gate?) do you have examples of closed threadchannels?

(removing close() would even simplify the implementation, as the thread channel is born open (it's not a state based on something) and the only thing that can make closed=true is the close() message...)

@NickHardeman
Copy link
Contributor

I'm also not sure about the condition.notify_all() ....

About the open/close, I am thinking specifically of ofxOscReceiver as outlined here( #7938

The osc thread would continue to try and push data to the thread channel ( when there was a lot of messages ) and we occasionally got a crash when deleting the shared_ptr to an ofxOscReceiver.
I think calling stop() should have the close method so that the stop is immediate, blocks the other thread (if any) and will reject any new messages.

//--------------------------------------------------------------
void ofxOscReceiver::stop() {
	messagesChannel.clear();
	messagesChannel.close();
	listenSocket.reset();
}

Our implementation for the project in thread channel was

void open() {
	std::unique_lock<std::mutex> lock(mutex);
	closed = false;
	condition.notify_all();
}
void clear() {
	std::unique_lock<std::mutex> lock(mutex);
	std::queue<T> empty;
   	std::swap( queue, empty );
	condition.notify_all();
}

@NickHardeman
Copy link
Contributor

Sorry for getting off topic of the PR, maybe we can carry on the open / close conversation here: #7938

This PR looks good to me 👍

@NickHardeman NickHardeman merged commit 62feca7 into openframeworks:master May 15, 2024
9 of 11 checks passed
danoli3 added a commit to danoli3/openFrameworks that referenced this pull request May 20, 2024
…leedingmacOS

* commit '7f37e70f65e9e022ba8868fb555570ce2c78a6ba': (37 commits)
  Allows Retina hi res enabled via App or Project.xcconfig (openframeworks#7971)
  actions changes (openframeworks#7968)
  Changing exr to hdr files for compatability with windows (openframeworks#7964)
  ofMesh - newfaces push_back to insert a list (openframeworks#7772)
  restore default-copy-constructibility of ofEvent (openframeworks#7969)
  [actions] ccache update (openframeworks#7967)
  Core small changes (openframeworks#7952)
  config.emscripten.default.mk for Emscripten >= 3.1.52 (openframeworks#7909)
  Fix edge case in findDelimiter (openframeworks#7911)
  oscpack / udpSocket: invert the "break_" semaphore (openframeworks#7963)
  ofxOscMessage: extra implicit adds [fixes something noted through openframeworks#7938 debugging] (openframeworks#7953) #changelog #ofxOsc
  ofThreadChannel::clear() to clear the channel (openframeworks#7921) #changelog #threadChannel
  OfxOscReceiver: from detach() to join() (openframeworks#7949)
  Update ofMathConstants.h (openframeworks#7958)
  [actions] update ubuntu 24.04 (openframeworks#7955)
  ofScopedMatrix (openframeworks#7946)
  [actions] - testing one action with multiple jobs for tests (openframeworks#7860)
  adding of.entitlements and vscode files to gitignore (openframeworks#7031)
  Make - use relative paths (openframeworks#7519)
  FPS timing with chrono (openframeworks#7867)
  ...

# Conflicts:
#	libs/openFrameworks/sound/ofAVEngineSoundPlayer.mm
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.

None yet

3 participants