Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5b393052 authored by Andre Eisenbach's avatar Andre Eisenbach
Browse files

Properly reset NONBLOCK flag in semaphore_try_wait() 

Without this fix, calling semaphore_try_wait() on a semaphore that
wasn't currently set, would leave the NONBLOCK flag on the file
descriptor as a side-effect.

Also added a unit test for semaphores, including a test specifically for
this condition.

Change-Id: I0ea37bb68b14c76febaab25b3aee1bb4f5acee8c
parent 539957df

system/osi/Android.mk

+1 −0
Original line number Diff line number Diff line
@@ -67,6 +67,7 @@ btosiCommonTestSrc := \ 
    ./test/list_test.cpp \

    ./test/reactor_test.cpp \

    ./test/ringbuffer_test.cpp \

    ./test/semaphore_test.cpp \

    ./test/thread_test.cpp \

    ./test/time_test.cpp

system/osi/src/semaphore.c

+4 −3
Original line number Diff line number Diff line
@@ -85,13 +85,14 @@ bool semaphore_try_wait(semaphore_t *semaphore) { 
    return false;

  }



  bool rc = true;

  eventfd_t value;

  if (eventfd_read(semaphore->fd, &value) == -1)

    return false;

    rc = false;



  if (fcntl(semaphore->fd, F_SETFL, flags) == -1)

    LOG_ERROR(LOG_TAG, "%s unable to resetore flags for semaphore fd: %s", __func__, strerror(errno));

  return true;

    LOG_ERROR(LOG_TAG, "%s unable to restore flags for semaphore fd: %s", __func__, strerror(errno));

  return rc;

}



void semaphore_post(semaphore_t *semaphore) {

system/osi/test/semaphore_test.cpp

0 → 100644
+87 −0
Original line number Diff line number Diff line 
#include <gtest/gtest.h>



#include "AllocationTestHarness.h"



extern "C" {

#include <unistd.h>

#include <sys/select.h>



#include "osi/include/osi.h"

#include "osi/include/reactor.h"

#include "osi/include/semaphore.h"

#include "osi/include/thread.h"

}



struct SemaphoreTestSequenceHelper {

  semaphore_t *semaphore;

  int counter;

};



namespace {

  void sleep_then_increment_counter(void *context) {

    SemaphoreTestSequenceHelper *helper =

            reinterpret_cast<SemaphoreTestSequenceHelper*>(context);

    assert(helper);

    assert(helper->semaphore);

    sleep(1);

    ++helper->counter;

    semaphore_post(helper->semaphore);

  }

} // namespace



class SemaphoreTest : public AllocationTestHarness {};



TEST_F(SemaphoreTest, test_new_simple) {

  semaphore_t *semaphore = semaphore_new(0);

  ASSERT_TRUE(semaphore != NULL);

  semaphore_free(semaphore);

}



TEST_F(SemaphoreTest, test_new_with_value) {

  semaphore_t *semaphore = semaphore_new(3);

  ASSERT_TRUE(semaphore != NULL);



  EXPECT_TRUE(semaphore_try_wait(semaphore));

  EXPECT_TRUE(semaphore_try_wait(semaphore));

  EXPECT_TRUE(semaphore_try_wait(semaphore));

  EXPECT_FALSE(semaphore_try_wait(semaphore));



  semaphore_free(semaphore);

}



TEST_F(SemaphoreTest, test_try_wait) {

  semaphore_t *semaphore = semaphore_new(0);

  ASSERT_TRUE(semaphore != NULL);



  EXPECT_FALSE(semaphore_try_wait(semaphore));

  semaphore_post(semaphore);

  EXPECT_TRUE(semaphore_try_wait(semaphore));

  EXPECT_FALSE(semaphore_try_wait(semaphore));



  semaphore_free(semaphore);

}



TEST_F(SemaphoreTest, test_wait_after_post) {

  semaphore_t *semaphore = semaphore_new(0);

  ASSERT_TRUE(semaphore != NULL);

  semaphore_post(semaphore);

  semaphore_wait(semaphore);

  semaphore_free(semaphore);

}



TEST_F(SemaphoreTest, test_ensure_wait) {

  semaphore_t *semaphore = semaphore_new(0);

  ASSERT_TRUE(semaphore != NULL);

  thread_t *thread = thread_new("semaphore_test_thread");

  ASSERT_TRUE(thread != NULL);



  EXPECT_FALSE(semaphore_try_wait(semaphore));

  SemaphoreTestSequenceHelper sequence_helper = {semaphore, 0};

  thread_post(thread, sleep_then_increment_counter, &sequence_helper);

  semaphore_wait(semaphore);

  EXPECT_EQ(sequence_helper.counter, 1) <<

      "semaphore_wait() did not wait for counter to increment";



  semaphore_free(semaphore);

  thread_free(thread);

}

system/osi/test/thread_test.cpp

+0 −1
Original line number Diff line number Diff line
@@ -6,7 +6,6 @@ extern "C" { 
#include <sys/select.h>



#include "osi/include/reactor.h"

#include "osi/include/semaphore.h"

#include "osi/include/thread.h"

#include "osi/include/osi.h"

}