demand() on channel from errorneous thread -> deadlock

[grump]

Code: Select all

io.stdout:setvbuf('no')
function love.threaderror(thread, errorStr)
	error(errorStr)
end

local thread = love.thread.newThread([[
	local channel = love.thread.getChannel('channel')
	channel.supply('hello')
]])

local channel = love.thread.getChannel('channel')
thread:start()
print(channel:demand())

The purpose of this program is to either print 'hello', or abort with an error message. Note that there's a deliberate bug in line 8: it should say channel:supply, not channel.supply.

When you run this program, nothing happens. It doesn't print hello, and it doesn't show any errors. It also becomes completely non-responsive. The task must be killed in order to quit.

I believe this is because the main thread reaches channel:demand() before the thread encounters the error. The main thread is now blocked, waiting for data on the channel; the thread can't supply data because of the error; love.threaderror doesn't get called because the main thread is blocked. Total deadlock. Removing the last line makes the error appear, confirming my theory. Sleeping for a bit before the demand() also makes it work as expected.

The demand() call directly after starting the thread is crucial to my design. I need the worker thread to send a list of identifiers that the main thread requires to continue.

A completely frozen app in case of a thread error is pretty unfortunate. I'd appreciate some ideas/advice on how to solve this problem in a smart way, without sleeps and without resorting to busy polling of the channel.

[bartbes]

I believe this is because the main thread reaches channel:demand() before the thread encounters the error. The main thread is now blocked, waiting for data on the channel; the thread can't supply data because of the error; love.threaderror doesn't get called because the main thread is blocked. Total deadlock. Removing the last line makes the error appear, confirming my theory. Sleeping for a bit before the demand() also makes it work as expected.

Note that with this code it will always block before the error is dispatched, because love.threaderror is called from the event loop. Your "sleep" must therefore not have been a love.timer.sleep.

A completely frozen app in case of a thread error is pretty unfortunate. I'd appreciate some ideas/advice on how to solve this problem in a smart way, without sleeps and without resorting to busy polling of the channel.

Since Channels and Threads are completely separate, there's no real way to do this cleanly. In 0.11.0 Channel:demand will have a timeout parameter, which could help. I think the best solution in 0.10.x is to try to catch the error in the thread using pcall/xpcall.

[grump]

Note that with this code it will always block before the error is dispatched, because love.threaderror is called from the event loop. Your "sleep" must therefore not have been a love.timer.sleep.

You're right, I did not test the sleep theory with this exact code. It's been a while since I tested it and I can't remember the details. There was one situation when love.timer.sleep() seemed to help, but the actual code this occurs in is much more complex of course.

I thought of love.threaderror as an exception handler, did not realize it's just an event.

I think the best solution in 0.10.x is to try to catch the error in the thread using pcall/xpcall.

Ugh, I was afraid of that answer Have all demand return values checked if there was an error or not... okay. That makes things much more complicated. It can't be easily unified into something like

Code: Select all

channel:supply({ error = true/false, data = ... })

when data is a table.

Edit: can you give some advice what is a good way to communicate errors to the main thread? Nested tables aren't allowed for some reason, and you can only push one value at a time,making this difficult to do efficiently. So the only way I can think of right now is to always push an error status first, then push the actual data or error message.

[bartbes]

Because of a bug you can actually push nested tables (though it won't detect cycles) in 0.10.2. Otherwise, if the returned data can be arbitrary you can really only do a double push (or use a second channel). If the data is a specific type you can return an error directly, or nil, or something like that.

[grump]

Because of a bug you can actually push nested tables (though it won't detect cycles) in 0.10.2.

I guess I shouldn't rely on this behavior because it will be fixed in the next version?

The decision not to allow nested tables creates a really tough restriction. It makes simple things like a thread that receives data through REST requests and passes readily parsed responses much harder to implement. You're basically being forced to do the JSON parsing in the main thread if you want to display that data.

Even trivialities like passing a status together with data, while being agnostic of the type of the data, suddenly get complicated because of that limitation. Using multiple push/pop calls might be possible, but it's not a good solution at all.

Can't you lift the restriction?

[bartbes]

It's already been lifted for 0.11.0, and it will error if it detects cycles (instead of go into an infinite loop/overflow the stack). It's still a very basic copying mechanism though, and I wouldn't be surprised if some lua serializers outperform it.

[grump]

It's already been lifted for 0.11.0, and it will error if it detects cycles (instead of go into an infinite loop/overflow the stack). It's still a very basic copying mechanism though, and I wouldn't be surprised if some lua serializers outperform it.

Nice. I for one am fine with that. I honestly prefer if it just works out of the box, even though some library might do a better job. Fewer libraries means less complexity and less room for errors, and if I really need the optimum solution, I can still use the lib.

This problem is solved, thanks.

[grump]

This problem is solved, thanks.

That was a bit of a premature statement. pcall works for runtime errors, but there's still no way to report syntax errors when using demand() immediately after starting the thread.

Code: Select all

local thread = love.thread.newThread([[
	local channel = love.thread:getChannel('channel')

	local ok, err = pcall(function()
		invalid_channel:supply({ error = false, data = 'hello' })
		syntax error here
	end)

	if not ok then
		print('Thread error')
		channel:supply({ error = true, data = err })
	end
]])

thread:start()
local result = love.thread.getChannel('channel'):demand()
print(result.error, result.data)

Freezes with no error message and no output.

Tried to solve it by loading the code with love.filesystem.load, but love.thread.newThread does not accept chunks. Now I'm loading it twice, once with love.filesystem.load to do the syntax check, then again in newThread.

[bartbes]

Yeah, the only way around that is to load the thread contents later, I guess. Something like:

Code: Select all

local channel, contents = ...
local ok, err = pcall(function()
    loadstring(contents)(channel)
end)
if not ok then
    channel:supply{error = true, data = err}
end

(Or pass a filename and call love.filesystem.load.)

[grump]

I solved it by using a stub that loads the actual code with love.filesystem.load from inside the thread, like you suggested. "We can solve any problem by introducing an extra level of indirection." (David J. Wheeler)

Is it possible, for a future version, to implement a mechanism for reporting thread errors that works like an exception instead of an event? Or maybe a way to bind channels to threads and make them release their block when the thread isn't running anymore. The timeout in 0.11 is an improvement, but still not ideal.