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

ofxOscReceiver Error when creating and destroying repeatedly #7938

Closed
NickHardeman opened this issue May 8, 2024 · 10 comments
Closed

ofxOscReceiver Error when creating and destroying repeatedly #7938

NickHardeman opened this issue May 8, 2024 · 10 comments

Comments

@NickHardeman
Copy link
Contributor

NickHardeman commented May 8, 2024

For a previous project we needed to create and destroy shared_ptrs of ofxOscSenders and ofxOscReceivers. When allocating and destroying quickly, sometimes we would get a crash.
We added a messagesChannel.clear() method just like #7921 to ofThreadChannel and called that along with messagesChannel.close() in the stop() method. This worked for our purposes because we were resetting the shared_ptrs and thus the stop method was getting called in the de-constructor.

Maybe the messagesChannel.close() should be in the de-constructor or ofxOscReceiver::stop()

//--------------------------------------------------------------
void ofxOscReceiver::stop() {
	messagesChannel.clear();
	messagesChannel.close();
	listenSocket.reset();
}
@artificiel
Copy link
Contributor

I made a little test app and rapid make_shared of ofxOscReceiver I get crashes in regards to ofxOscReceiver::listenSocket — which points to the internal OSC thread. I don't know if it's the same behaviour you have, it must depend on specific parameters, but there might be ambiguity there (perhaps as the thread is .detached())

brainstorming here but it seems the SocketReceiveMultiplexer::AsynchronousBreak() is designed to be tread safe (basically sending a teardown message to the oscpack socket processor through the socket itself). adding that into ofxOscReceiver::stop(), as well as an atomic semaphore to indicate the break is really done at the end of SocketReceiveMultiplexer::Implementation::Run() makes it more solid. in short: notify the socket processor that listening must stop, wait until Run is done; then complete destruction.

but it's only an hypothesis and needs a bit more thought (I implemented roughly with atomic bool).

@artificiel
Copy link
Contributor

i made a crashing app here #7949; it's probably not the same crash you get, but it's a crash related to deallocation of ofxOscReceiver. I currently am implicated in macOS only; if you can try it on windows just to confirm the crashing behaviour there.

I have a fix for it, but it requires minor modding to oscpack's udpsocket implementation: changing the break_ from an old-school volatile to a proper atomic, and provide a getter to be able to retrieve it. this is then used in the ofxOscReceiver::close to not reset the pointer if break_ is true (I think it is because it might be in deallocation in the other thread from the custom deleter (so double-delete occurs), but it's not easy to track). I also lock a shared_mutex around listenThread vs close() (so close() waits for the Run() thing to be done before closing) and use listenSocket->AsynchronousBreak() to close the socket through itself. actually maybe the custom deleter is in play, the comments says:

	// detach thread so we don't have to wait on it before creating a new socket
	// or on destruction, the custom deleter for the socket unique_ptr already
	// does the right thing

but maybe "the right thing" was not tested in extreme alloc/dealloc with "overlapping" instances.

so please let me know if you get crashes, and if you want to add crashing scenarios that are closer to your behaviour please go for it. I will ponder a bit more with some distance before submitting a pull request for the fix and we can discuss the logic more at that point.

@NickHardeman
Copy link
Contributor Author

I get a crash immediately using your test with the following code. This is without any modifications you mentioned to ofxOsc. Did you push any changes to ofxOscReceiver you made for testing?

//--------------------------------------------------------------
void ofApp::setup(){
	ofxOscMessage m { "/test" };
	m.add("something", "else", 20.3f);
	
	ofxOscSender sender { "127.0.0.1", 2002 };
	std::shared_ptr<ofxOscReceiver> receiver;
	
	receiver = std::make_shared<ofxOscReceiver>(2002);
	sender.send(m);
	receiver = std::make_shared<ofxOscReceiver>(2003);
	receiver = std::make_shared<ofxOscReceiver>(2003);
}

@NickHardeman
Copy link
Contributor Author

I'm actually getting the crash just from making a message:

//--------------------------------------------------------------
void ofApp::update(){
	if( ofGetFrameNum() == 3 ) {
		ofxOscMessage m { "/test" };
		m.add("something", "else", 20.3f);
		
	}
}

@NickHardeman
Copy link
Contributor Author

When I change to this, then no crash, I am using a nightly from a week or two ago.

	if( ofGetFrameNum() == 3 ) {
		ofxOscMessage m;
		m.setAddress("/test");
		m.addStringArg("something");
		m.addStringArg("else");
		m.addFloatArg(20.3f);
	}

@artificiel
Copy link
Contributor

hmm i will look i to the « add » method (it’s a different thing no code was pushed) but did you manage to run the app in #7949?

please confirm, and i will push a fix proposal there, so we can be a bit systematic — maybe you can look in the app: i’m doing funny stuff, but perhaps not the same funny you have in your case. if you can reproduce your case in the app (perhaps ifdef’d) it would be great to test the fixed with as many cases.

@NickHardeman
Copy link
Contributor Author

I just tried with the test app in #7949 and it hung, and eventually crashed. I am using the latest nightly and macOS 14.2.1.
In the Xcode console I see
[notice ] ready to roll:

In the "Problem Report" The exception type is EXC_BAD_ACCESS (SIGSEGV) and outputs.

0   TestOscReceiverDebug          	       0x102293638 ofxOscMessage& ofxOscMessage::add<unsigned long>(unsigned long) + 24 (ofxOscMessage.h:178)
1   TestOscReceiverDebug          	       0x102293644 ofxOscMessage& ofxOscMessage::add<unsigned long>(unsigned long) + 36 (ofxOscMessage.h:179)
2   TestOscReceiverDebug          	       0x102293644 ofxOscMessage& ofxOscMessage::add<unsigned long>(unsigned long) + 36 (ofxOscMessage.h:179)
3   TestOscReceiverDebug          	       0x102293644 ofxOscMessage& ofxOscMessage::add<unsigned long>(unsigned long) + 36 (ofxOscMessage.h:179)
4   TestOscReceiverDebug          	       0x102293644 ofxOscMessage& ofxOscMessage::add<unsigned long>(unsigned long) + 36 (ofxOscMessage.h:179)
.......... 

When I use addIntArg(), etc , then I see the OF window with colored rectangles on the screen and then it crashes, pointing to different places.
Xcode output

[notice ] ready to roll: 
[warning] ofxOscReceiver: select failed

[warning] ofxOscReceiver: select failed
...........

Wrapped in an ifdef.

for (const auto & tester: testers_) {
	for (size_t i=0; i<10; i++) {
			
#if defined(USE_EXPLICIT_FUNCTION)
		ofxOscMessage m;
		m.setAddress("/test");
		m.addIntArg(std::int32_t(i));
		m.addIntArg(port_);
		m.addIntArg(i);
		tester->sender_->sendMessage(m);
#else
		ofxOscMessage m{"/test"};
		m.add(i);
		tester->sender_->sendMessage(m.add(port_).add(i));
#endif
			
		msgs_out_++;
	}
} 

@artificiel
Copy link
Contributor

artificiel commented May 13, 2024

OK just to keep things sane I'm continuing in the PR thread #7949

@artificiel
Copy link
Contributor

#7953 fixes the non-explicit message (was choking on the const char * arg)

NickHardeman pushed a commit that referenced this issue May 15, 2024
… debugging] (#7953) #changelog #ofxOsc

* ofxOscMessage::add(size_t argument)

* ofxOscMessage::add(const char * argument)
@NickHardeman
Copy link
Contributor Author

Closed by #7949

danoli3 added a commit to danoli3/openFrameworks that referenced this issue 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

No branches or pull requests

2 participants