LabRecorder deadlocks writing a streams footer, if it has not yet received anything from this stream about app-labrecorder HOT 10 CLOSED

Hey I just uncovered this bug myself, the problem is that the calls to pull_sample and pull_chunk_multiplexed do not provide a timeout parameter, therefore they go by the LSL default which is "FOREVER". Therefore the typed_transfer_loop never checks or re-checks the shutdown flag, so the thread doesn't finish properly. Obviously if you actually have data for the inlet to pull, the code won't hang on the pull calls, but for a "dead" stream, it just keeps waiting.

recording.cpp typed_transfer_loop
first_timestamp = last_timestamp = in->pull_sample(chunk);
		timestamps.push_back(first_timestamp);
		file_.write_data_chunk(streamid, timestamps, chunk, in->get_channel_count());

		auto next_pull = Clock::now();
		while (!shutdown_) {
			// get a chunk from the stream
			in->pull_chunk_multiplexed(chunk, &timestamps);
			// for each sample...
			for (double &ts : timestamps) {
// if the time stamp can be deduced from the previous one...

from app-labrecorder.

Definitely a bug, I was already looking into something related to this but a stream without data is a lot easier to reproduce.

from app-labrecorder.

Thanks @kowalej, that's exactly what I was looking for. I've commited a fix in fded925 and will upload binaries for that soon

from app-labrecorder.

@tstenner I just finished doing a PR for the console message stuff and noticed that your commit fded925 has a potential bug.

//recording.cpp Line 377
while(first_timestamp == 0.0)
	first_timestamp = last_timestamp = in->pull_sample(chunk, 4.0); 

Since you are in a while loop and pull_sample keeps returning 0.0, you will get stuck in this loop during shutdown since you don't also check the shutdown state.

It should be:

while (!shutdown_ && first_timestamp == 0.0)
	first_timestamp = last_timestamp = in->pull_sample(chunk, 4.0); 

from app-labrecorder.

You're right, I fixed it.

from app-labrecorder.

We're still getting this bug.
Also the status bar kb-written seems to be broken.

from app-labrecorder.

@cboulay I've fixed the status bar kb-written bug:

//mainwindow.cpp line 91 should be
auto fileinfo = QFileInfo(ui->rootEdit->text()+QDir::separator()+recFilename);

from app-labrecorder.

@xbbsky That didn't do much for me because it turns out that the file size wasn't incrementing. When I reduced the timeout parameter then everything seemed to fall in place. Thanks for your comment on that commit.

from app-labrecorder.

Maybe there's still some discussion to be had as to what the timeout parameter should be, but the bug as described in this issue is fixed.

from app-labrecorder.

I also experienced this issue again recently and setting a low value for timeout definitely fixed it. However, I don't really understand why it fixed the issue since the timeout was only a few seconds previously. Also, I may just be crazy but I remember the previous fix (with checking the shutdown parameter) as having resolved this issue for me before the holiday. Now it's a few weeks later and my recent tests weren't shutting down properly. It's like this issue cropped up again out of the blue.

from app-labrecorder.