Skip to content

CanUSE_PTHREADS and MINIMAL_RUNTIME exist at the same time? #12578

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

Closed
tigercosmos opened this issue Oct 22, 2020 · 6 comments
Closed

CanUSE_PTHREADS and MINIMAL_RUNTIME exist at the same time? #12578

tigercosmos opened this issue Oct 22, 2020 · 6 comments

Comments

@tigercosmos
Copy link

tigercosmos commented Oct 22, 2020

CanUSE_PTHREADS and MINIMAL_RUNTIME exist at the same time?

I have a simple pthread code test.c.

I run em with the following arguments:

emcc test.c \
		-s USE_PTHREADS=1 \
		-s PTHREAD_POOL_SIZE=4 \
		-s ENVIRONMENT=web,worker \
		-s MINIMAL_RUNTIME \
		-O3 \
		-o test.html

Then I got an error:

The code works well when I don't set `MINIMAL_RUNTIME`.

I use version 2.0.7.

test.c looks like:

#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>

#define NUMTHRDS 4
#define MAGNIFICATION 1e9

typedef struct
{
    double *sum;
} Arg;

pthread_t callThd[NUMTHRDS];

void *count_pi(void *arg)
{

    Arg *data = (Arg *)arg;
    double *sum = data->sum;
    *sum += 1;

    printf("sum=%f\n", *sum);

    pthread_exit((void *)0);
}

int main(int argc, char *argv[])
{
    pthread_attr_t attr;
    pthread_attr_init(&attr);
    pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE);

    double *sum = malloc(sizeof(*sum));
    *sum = 0;

    Arg arg[NUMTHRDS];
    for (int i = 0; i < NUMTHRDS; i++)
    {
        arg[i].sum = sum;
        pthread_create(&callThd[i], &attr, count_pi, (void *)&arg[i]);
    }

    pthread_attr_destroy(&attr);

    void *status;
    for (int i = 0; i < NUMTHRDS; i++)
    {
        pthread_join(callThd[i], &status);
    }

    printf("Final Sum =  %f \n", *sum);

    free(sum);
}
@tigercosmos
Copy link
Author

tigercosmos commented Oct 22, 2020

I found the problem:

it's because there is inrelevant code not been deleted:

function _emscripten_futex_wait(addr, val, timeout) {
    if (addr <= 0 || addr > HEAP8.length || addr & 3 != 0) return -28;
    if (!ENVIRONMENT_IS_WEB) {
        var ret = Atomics.wait(HEAP32, addr >> 2, val, timeout);
        if (ret === "timed-out") return -73;
        if (ret === "not-equal") return -6;
        if (ret === "ok") return 0;
        throw "Atomics.wait returned an unexpected value " + ret
    } else {

Same problem here, no need ENVIRONMENT_IS_NODE:

function _pthread_exit(status) {
    if (!ENVIRONMENT_IS_PTHREAD) _exit(status);
    else PThread.threadExit(status);
    if (ENVIRONMENT_IS_NODE) {
        process.exit(status)
    }
    throw "unwind"
}

I can help open a PR, but I would need some hints!

@kripken
Copy link
Member

kripken commented Oct 23, 2020

cc @juj

Yes, I think this is not done yet, and PRs would be great of course!

Is the issue that ENVIRONMENT_IS_WEB etc. are not defined? I'm not sure offhand how best to fix that. If it's a tiny ifdef maybe that's ok, but long-term we should be able to avoid all of this using #12203

@tigercosmos
Copy link
Author

Yes, ENVIRONMENT_IS_WEB and ENVIRONMENT_IS_NODE are not defined in the output code, but they also should not exist at all.

I fix the issue by

	emcc test.c \
		-s USE_PTHREADS=1 \
		-s PTHREAD_POOL_SIZE=4 \
		-s ENVIRONMENT=web,worker \
		-s MINIMAL_RUNTIME \
		-O3 \
		-o test.html
	mv test.js tmp.js
	sed 's/ENVIRONMENT_IS_NODE/false/g' tmp.js | \
	sed 's/ENVIRONMENT_IS_WEB/true/g' > test.js

@stale
Copy link

stale bot commented Apr 17, 2022

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 30 days. Feel free to re-open at any time if this issue is still relevant.

@stale stale bot added the wontfix label Apr 17, 2022
@tigercosmos
Copy link
Author

@kripken I haven't checked this issue for a long time. Not sure if this issue still an issue.

@stale stale bot removed the wontfix label Apr 18, 2022
@kripken
Copy link
Member

kripken commented Apr 18, 2022

I believe this should be fixed, as we have tests for it now.

@kripken kripken closed this as completed Apr 18, 2022
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

2 participants