[ft6206_async] Device is not closed unless a callback is specified

[stc1988]

Build environment: macOS
Moddable SDK version: 8.2.0
Target device: M5StackCoreS3

Description

If a callback is not specified, io.close is not called. Is this the intended behavior?

[stc1988]

Also, assuming that close is processed asynchronously, I’m concerned about the following two points.

• It would be better to stop the timer process that reads from the sensor first.
• Release the #io resource only after close has completed.

The implementation I’m considering is as follows.

	close(callback) {
		this.#io?.reset?.close();
		this.#io?.interrupt?.close();
		Timer.clear(this.#timer);
		this.#timer = undefined;
		if (this.#io){
			this.#io.close(error => {
				callback?.(error)
				this.#io = undefined;
			});
		}
	}

[phoddie]

Thank you for the careful review.

There is definitely a bug that the #io.close is skipped if there's no callback.

I don't think when the timer is cleared is significant. Because JavaScript is strictly single-threaded, the timer, reset, and interrupt cannot invoke their callbacks during execution of the FT6206's close(). If they fire while it is executing, that callback will never be invoked because when the #timer, reset, and interrupt are closed/cleared any pending callbacks are canceled.

Similarly, this.#io can be reset during close() because nothing in the ft6206 driver should access it after that.

So, I think maybe this variation of your proposal is enough – but I don't have a device with me to test, so please tell me if I am wrong. ;)

	close(callback) {
		this.#io?.reset?.close();
		this.#io?.interrupt?.close();
		this.#io?.close(error => callback?.(error));
		this.#io = undefined;
		Timer.clear(this.#timer);
		this.#timer = undefined;
	}

FWIW – asynchronous close() is often a source of bugs. It is trickier go get right than people (me!) assume. And close() is often not invoked since driver may just be opened at launch and run until the device is reset.

[stc1988]

Thanks for the detailed explanation. I understand that my suggestion wouldn’t be that effective.

FWIW – asynchronous close() is often a source of bugs. It is trickier go get right than people (me!) assume. And close() is often not invoked since driver may just be opened at launch and run until the device is reset.

In Stack-chan, there is case where the touch driver invokes close().

To accommodate this release change to asynchronous behavior, I updated the code to use the close callback. However, a panic occurs during processing after close(). Since the issue does not occur with the synchronous touch driver, I think there may be a problem with the asynchronous close() implementation.

Guru Meditation Error: Core  0 panic'ed (LoadProhibited). Exception was unhandled.

Core  0 register dump:
PC      : 0x42177148  PS      : 0x00060630  A0      : 0x82063b1c  A1      : 0x3fced250
--- 0x42177148: fxForget at /Users/satoshi/Projects/moddable/xs/sources/xsAPI.c:1863
--- (inlined by) fxForget at /Users/satoshi/Projects/moddable/xs/sources/xsAPI.c:1853
A2      : 0x3fcedb00  A3      : 0x3fcc4ddc  A4      : 0x00060620  A5      : 0x3fc9f460
A6      : 0x403769f0  A7      : 0x00060623  A8      : 0x00220008  A9      : 0x00220008
--- 0x403769f0: serial_isr at /Users/satoshi/Projects/moddable/modules/io/serial/esp32/serial.c:371
A10     : 0x00060623  A11     : 0x00000000  A12     : 0x00060620  A13     : 0x0000000e
A14     : 0x00000011  A15     : 0x00000000  SAR     : 0x0000000f  EXCCAUSE: 0x0000001c
EXCVADDR: 0x00220008  LBEG    : 0x400570e8  LEND    : 0x400570f3  LCOUNT  : 0x00000000
--- 0x400570e8: memset in ROM
--- 0x400570f3: memset in ROM

Backtrace: 0x42177145:0x3fced250 0x42063b19:0x3fced270 0x4204fee5:0x3fced290 0x420a51cd:0x3fced300 0x42081d01:0x3fced3a0 0x4208201d:0x3fced440 0x420a51cd:0x3fced460 0x4209c686:0x3fced500 0x420a51cd:0x3fced5a0 0x4209f07d:0x3fced640 0x4200ff2d:0x3fced6d0 0x42010389:0x3fced6f0 0x4200f9c3:0x3fced720 0x42185d1e:0x3fced740
--- 0x42177145: fxForget at /Users/satoshi/Projects/moddable/xs/sources/xsAPI.c:1861
--- (inlined by) fxForget at /Users/satoshi/Projects/moddable/xs/sources/xsAPI.c:1853
--- 0x42063b19: fxRemember at /Users/satoshi/Projects/moddable/xs/sources/xsAPI.c:1848
--- 0x4204fee5: xs_serial_constructor at /Users/satoshi/Projects/moddable/modules/io/serial/esp32/serial.c:224
--- 0x420a51cd: fxRunID at /Users/satoshi/Projects/moddable/xs/sources/xsRun.c:891
--- 0x42081d01: fxStepAsync at /Users/satoshi/Projects/moddable/xs/sources/xsFunction.c:675
--- 0x4208201d: fxResolveAwait at /Users/satoshi/Projects/moddable/xs/sources/xsFunction.c:640
--- 0x420a51cd: fxRunID at /Users/satoshi/Projects/moddable/xs/sources/xsRun.c:891
--- 0x4209c686: fxOnResolvedPromise at /Users/satoshi/Projects/moddable/xs/sources/xsPromise.c:536
--- 0x420a51cd: fxRunID at /Users/satoshi/Projects/moddable/xs/sources/xsRun.c:891
--- 0x4209f07d: fxRunPromiseJobs at /Users/satoshi/Projects/moddable/xs/sources/xsPromise.c:1294
--- 0x4200ff2d: doRunPromiseJobs at /Users/satoshi/Projects/moddable/xs/platforms/esp/xsHost.c:1344
--- 0x42010389: modMessageService at /Users/satoshi/Projects/moddable/xs/platforms/esp/xsHost.c:1097
--- 0x4200f9c3: loop_task at /Users/satoshi/Projects/moddable/build/tmp/esp32/m5stack_cores3/instrument/stackchan/xsProj-esp32s3/main/main.c:111
--- 0x42185d1e: vPortTaskWrapper at /Users/satoshi/.local/share/esp32/esp-idf-v6.0/components/freertos/FreeRTOS-Kernel/portable/xtensa/port.c:143

[phoddie]

In Stack-chan, there is case where the touch driver invokes close().

Interesting. It seems to be some kind of override for launching the mod?

The native stack trace doesn't give any clues – and may contain some red herrings such as xs_serial_constructor. Debugging inside Stack-chan could be impractical. Would you kindly provide a small standalone example that demonstrates the problem?

[stc1988]

This is a small standalone example that demonstrates the problem.
touch.zip

[phoddie]

Thank you for the simple example. That's very helpful. And revealing. You included the invocation of the Serial constructor, which I wouldn't have guessed as my reading of the Stack-chan code focused on how it used asynchronous close(), not what followed. Perhaps the xs_serial_constructor is less of a red-herring than it first appeared.

[phoddie]

It looks like there's a missing call to xsForget(). I'm able to run the sample cleanly by changing...

moddable/modules/io/i2c/esp32/_i2c.c

Lines 571 to 572 in 52fe713

	if (kOperationClose == operation)
	_xs_i2casync_destructor(transaction->i2c); // all resources must be released before invoking callback ... so callback could call new again

to

	if (kOperationClose == operation) {
		xsForget(transaction->i2c->obj);
		_xs_i2casync_destructor(transaction->i2c);	// all resources must be released before invoking callback ... so callback could call new again
	}

Would you give that a try? If it works for you in your example, please also try in Stack-chan. Thank you!

[stc1988]

Would you give that a try? If it works for you in your example, please also try in Stack-chan. Thank you!

It works for my example and Stack-chan. Thank you.

Actually, with a recent update, Stack-chan has moved all touch handling over to Piu, so this issue no longer occurs. Still, I’m glad we were able to identify and fix the I2C problem.