basvandijk / concurrent-extra

Extra concurrency primitives
Other
17 stars 5 forks source link

concurrent-extra-0.7.0.9 test suite failure #12

Open peti opened 9 years ago

peti commented 9 years ago

Citing from http://hydra.cryp.to/build/1114863/nixlog/1/raw:

Running 1 test suites...
Test suite test-concurrent-extra: RUNNING...
Pessimistic locking:
  Event:
    set wait a: [OK]
    set wait b: [OK]
    set wait c: [OK]
    set wait d: [OK]
    conc set wait: [OK]
    multi wake: [OK]
    exception: [OK]
    wait timeout: [OK]
    wait blocks: [OK]
  Lock:
    acquire release: [OK]
    acquire acquire: [OK]
    new release: [OK]
    new unlocked: [OK]
    newAcquired locked: [OK]
    acq rel unlocked: [OK]
    conc release: [OK]
    wait: [OK]
  STM.Lock:
    acquire release: [OK]
    acquire acquire: [OK]
    new release: [OK]
    new unlocked: [OK]
    newAcquired locked: [OK]
    acq rel unlocked: [OK]
    conc release: [OK]
    wait: [OK]
  RLock:
    recursive acquire: [OK]
    conc acquire: [OK]
  ReadWriteLock:
    test1: [OK]
    test2: [OK]
    stressTest: [Failed]

         Test Cases   Total       
 Passed  29           29          
 Failed  1            1           
 Total   30           30          
Test suite test-concurrent-extra: FAIL
sjakobi commented 6 years ago

I also ran into this one.

DanBurton commented 6 years ago

Reproduced on the Stackage build server with concurrent-extra-7.0.12 using ghc-8.4.1.

  ReadWriteLock:
    test1: [OK]
    test2: [OK]
    stressTest: [Failed]

         Test Cases   Total
 Passed  29           29
 Failed  1            1
 Total   30           30
basvandijk commented 6 years ago

I don't have an idea yet why the stressTest for RWLocks is failing. Maybe the timeout of 500 * a_moment (2.5s) is too small.

In any case, I'm not happy with the implementation of RWLock. I think we can simplify the implementation by replacing the three MVars with some TVars. You typically loose fairness when doing that which is undesirable. However we already don't have fairness because we have a racy implementation. We race here, here and here.

@roelvandijk what do you think about switching to STM?

crockeea commented 5 years ago

@roelvandijk any progress on this ticket? I'm also getting this failure.