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

When multiple instances of Duktape are running concurrently console output/errors are saved to the wrong Duktape object #28

Open
jezwhite opened this issue Sep 20, 2021 · 2 comments

Comments

@jezwhite
Copy link

All,

I have multiple instances of Duktape in-flight controlled via an external event loop (Mojo). When one instance of Duktape is waiting for an event to complete (ie, fetching data from the web) console output and errors are written to the wrong Duktape instance (looks like the last created).

I assume this is a valid use case? Other than console output/errors, everything else seems ok.

The following example will show the issue:

   use warnings;
   use strict;
   use Data::Dumper;
   use JavaScript::Duktape::XS;

   my $options = {
     gather_stats     => 1,
     save_messages    => 1,
   };

   my $instanceOne = JavaScript::Duktape::XS->new($options);   
   my $instanceTwo = JavaScript::Duktape::XS->new($options);   
   $instanceOne->eval('console.log("hello from instance one")');

   print "output from instance one: \n";
   print Dumper $instanceOne->get_msgs();
   print "output from instance two: \n";
   print Dumper $instanceTwo->get_msgs();

When run, the output will be shown for $instanceTwo rather than $instanceOne that ran the code.

I believe the problem is with

typedef struct ConsoleConfig {
    ConsoleHandler* handler;
    void* data;
} ConsoleConfig;

In duk_Console.c

As it's a global and not on an object basis, it's causing the problem. I think the solution would be to associate a pointer from the perl side (ie, Duk*) into the actual duktape (ie, ctx*) and remove the global struct above.

Thoughts?

Regards,

@gonzus
Copy link
Owner

gonzus commented Sep 22, 2021

This looks like a reasonable use case, and is not something I tried before. I will consider a PR (with tests please) that fixes this.

Cheers!

@jezwhite jezwhite mentioned this issue Oct 9, 2021
@gflohr
Copy link
Contributor

gflohr commented Jun 21, 2023

This has been with PR #29. I think the issue can be closed. I have a unit test for the issue here. If you want to add it, I will open a PR.

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

3 participants