svaarala / duktape

Duktape - embeddable Javascript engine with a focus on portability and compact footprint
MIT License
5.95k stars 515 forks source link

Relax multithreading requirements to allow multiple non-concurrent native threads #391

Closed svaarala closed 6 years ago

svaarala commented 9 years ago

Current threading requirement is that only a single native thread can call into Duktape and another thread may enter only when the previous one exits.

When implementing blocking native calls it'd be useful to relax this so that:

Thread B could equally well also call into a blocking native function and allow thread C to execute while that happens. This policy requires that the user code manage and exclude multiple threads accurately, e.g. by using an "execution permission" lock. In the example above thread A would obtain the lock before executing, and release the lock just before blocking. Duktape API calls are only allowed when holding the lock. The policy also implies that the Duktape threads involved in threads A and B (etc) never overlap.

Implementing this policy needs some internal state changes so that thread A can resume even when something has happened in the middle. For example, current code assumes heap->curr_thread which needs to be saved and restored or removed entirely.

Tasks to implement first version into Duktape 2.0:

svaarala commented 9 years ago

Since the user code will in most cases need to manage some kind of execution lock, it'd probably be quite OK to require user code to use some API calls to tell Duktape when the thread may change and when thread A resumes.

fatcerberus commented 9 years ago

So in this scenario, when Thread A resumes (the blocking call is satisfied), it would have to request the mutex back immediately?

svaarala commented 9 years ago

Yes, before making any Duktape API calls.

svaarala commented 9 years ago

But note that mutexes/locks are not required - the user code just has to guarantee that there are no simultaneous active calls into Duktape API. Mutexes/locks are probably the easiest way to do that, but some applications may be able to guarantee that without explicit locking too.

fatcerberus commented 9 years ago

I was never daring enough to try lockless programming. :)

svaarala commented 9 years ago

Hehe, I was more referring to cases where the program is a single threaded event loop which executes a callback until it blocks - and then resumes to handle another event. In this case there's actually just one native thread which alternates between multiple active Duktape threads. Locking would not be necessary because there's just one native thread.

andoma commented 9 years ago

FTR, Spidermonkey supports this via the JS_SuspendRequest() and JS_ResumeRequest() API

svaarala commented 9 years ago

Hmm, that's closely related but seems to a little bit different thing (e.g. garbage collection is never locked/held in Duktape)? It's still probably good reference if coming from Spidermonkey.

fatcerberus commented 9 years ago

Spidermonkey probably has to suspend GC in this case because it's done off-thread and may cause objects to be moved, potentially invalidating pointers in the process. Duktape actually has it a bit easier because it's single-threaded at the heap level and IIRC heap pointers are stable for the lifetime of the object.

andoma commented 9 years ago

Yeah, the threading infrastructure in SM was very complicated. Multiple native threads were allows to execute concurrently in the same runtime to some extent. I can't remember the details.

But running multiple threads was also buggy, sometimes it crashed more or less randomly and upgrading was not really an option for me as SM had grown to a big hairy beast.

This was why I gave up on SM and started to search for an alternative and luckily found Duktape.

fatcerberus commented 9 years ago

@svaarala Concretely, what is preventing this from being feasible right now, in master? I assume it's more complicated than just the curr_thread pointer.

svaarala commented 9 years ago

I'd have to look up my old notes on that, but it's just a bunch of small book-keeping things here and there, basically anything to do with heap level state which doesn't get stored/restored automatically. There are also some assumptions in call handling which caused issues but those have mostly been resolved by earlier reworks.

svaarala commented 7 years ago

Moving the final cleanup issue, improving the suspend/resume structure representation, to 2.1.0 because it can be made in a compatible fashion.

paolo-losi commented 6 years ago

@svaarala how complex is this? would you accept a bounty?

svaarala commented 6 years ago

@paolo-losi Could you clarify what feature you mean? The main feature tracked by this issue has already been implemented in the form of duk_suspend() and duk_resume() API calls. The only remaining issue is internal header declaration trivia which is not visible to the application.

paolo-losi commented 6 years ago

@svaarala I am referring to supporting the usecase that you outlined in the main comment. Can you confirm that duk_suspend() / duk_resume() are enough for supporting that usecase? If yes, I guess only internal cleanup prevents the bug to be closed, right?

svaarala commented 6 years ago

Yes, the suspend/resume use case is functional, and the only checkbox preventing closing of this issue is the header cleanup.

svaarala commented 6 years ago

Moved the remaining cleanup issue to #1847 for clarity.