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

PhTracedEnclosure.java:36-38: Make the class thread safe.... #2891

Closed
0pdd opened this issue Feb 22, 2024 · 4 comments
Closed

PhTracedEnclosure.java:36-38: Make the class thread safe.... #2891

0pdd opened this issue Feb 22, 2024 · 4 comments

Comments

@0pdd
Copy link

0pdd commented Feb 22, 2024

The puzzle 2836-88180ad7 from #2836 has to be resolved:

* @todo #2836:60min Make the class thread safe. It has private static
* field which can be accessed from differ thread and is not thread safe.
* Needs to synchronize this field.

The puzzle was created by Yegor Bugayenko on 22-Feb-24.

Estimate: 60 minutes, role: DEV.

If you have any technical questions, don't ask me, submit new tickets instead. The task will be "done" when the problem is fixed and the text of the puzzle is removed from the source code. Here is more about PDD and about me.

@levBagryansky
Copy link
Member

@volodya-lombrozo why don't you like ConcurrentHashMap here?
I think ThreadLocal could not help in scenario, something like that: Dataizing of cage's enclosure means firstly creating new thread, closing thread, and dataizing next in the new thread. So every next entry to cage would mean you should trace again.

@volodya-lombrozo
Copy link
Member

@levBagryansky I was afraid of the following case:

    @Test
    void dataizesManyCagesInParallel() {
        MatcherAssert.assertThat(
            "We have to be able to dataize many cages in parallel",
            Stream.generate(() -> (Supplier<Long>) this::dataizeOneMoreCage)
                .limit(1000)
                .parallel()
                .map(Supplier::get)
                .mapToLong(Long::longValue)
                .sum(),
            Matchers.equalTo(11000L)
        );
    }

    private long dataizeOneMoreCage() {
        final Phi cage = new EOcage(Phi.Φ);
        // Enable the "hack" method to break the test
        // this.hack();
        EOcageTest.writeTo(cage, new Data.ToPhi(11L));
        return new Dataized(cage).take(Long.class);
    }

    /**
     * How to break the test.
     * Here, I just reset the counter of vertices to 0.
     * The test above works only because {@link org.eolang.Vertices} is thread save and
     * smart enough. Each time when you create a new cage, it gets a new unique vertex. 
     * Exactly this behavior saves the test from failing.
     * But if somebody changes this logic, the test will fail. For example, if each dataization
     * process starts with zero vertex, the test will fail.
     * So even if you use {@link java.util.concurrent.ConcurrentHashMap}, it won't save you
     * in this scenario.
     */
    private void hack() {
        try {
            final Field vtxfield = PhDefault.class.getDeclaredField("VTX");
            vtxfield.setAccessible(true);
            final Object vtx = vtxfield.get(null);
            final Field count = vtx.getClass().getDeclaredField("count");
            count.setAccessible(true);
            final AtomicInteger atomic = (AtomicInteger) count.get(vtx);
            atomic.set(0);
        } catch (final NoSuchFieldException | IllegalAccessException exception) {
            throw new IllegalStateException(exception);
        }
    }

But it seems, Vertices saves us from it. The test above shows that using ConcurrentHashMap doesn't help in this particular scenario.

@levBagryansky
Copy link
Member

@volodya-lombrozo I see. ConcurrentHashMap requires different hashcodes for any different objects independently of thread. As for me this is a fundamental feature of eo hashcode. eo-threds requires this feature too.

levBagryansky added a commit to levBagryansky/eo that referenced this issue Mar 5, 2024
levBagryansky added a commit to levBagryansky/eo that referenced this issue Mar 5, 2024
levBagryansky added a commit to levBagryansky/eo that referenced this issue Mar 14, 2024
levBagryansky added a commit to levBagryansky/eo that referenced this issue Mar 14, 2024
levBagryansky added a commit to levBagryansky/eo that referenced this issue Mar 14, 2024
levBagryansky added a commit to levBagryansky/eo that referenced this issue Mar 14, 2024
levBagryansky added a commit to levBagryansky/eo that referenced this issue Mar 14, 2024
levBagryansky added a commit to levBagryansky/eo that referenced this issue Mar 14, 2024
levBagryansky added a commit to levBagryansky/eo that referenced this issue Mar 14, 2024
levBagryansky added a commit to levBagryansky/eo that referenced this issue Mar 14, 2024
@0pdd 0pdd closed this as completed Apr 6, 2024
@0pdd
Copy link
Author

0pdd commented Apr 6, 2024

The puzzle 2836-88180ad7 has disappeared from the source code, that's why I closed this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants