Added a per thread buffer and related tests for OMR socket API

[caohaley]

Created 4 commits for this PR:

• Added Omrsock Related Per Thread Buffer functions contains changes related to the per thread buffer and its functionalities.
• Added startup and shutdown functions to omrsock API contains changes related to OMR socket API startup and shutdown, which utilizes the ptb functions in the previous commit to startup and shutdown the per thread buffer.
• Added Unix implementation for omrsock_getaddrinfo_create_hints contains implementation of the OMR socket API function, that uses the ptb function to store information. It also contains a few new error codes related to OMR socket API.
• Added Implementations to omrsockTest.cpp to test omrsock PTB contains tests that are still being work on for the per thread buffer.

[caohaley]

@rwy0717 @babsingh
I ran into a few problems with this PR.

First, I need to include omrsock.h in omrsockTest.cpp to access elements of the struct OMRAddrInfoNode. However, I'm not sure how to include the file without specifying a path to it.

Also, when we run omrsockTest.cpp, for windows systems, it won't be able to find a omrsock.h because I haven't implemented for windows systems yet. It also will only read common/omrsock.c where the implementations will only return errors. It is possible to only run the tests on Unix systems? Should I just wrap the test code in per_thread_buffer_functionality test with #if defined (Unix) or something similar to that?

[babsingh]

Need to include omrsock.h in omrsockTest.cpp. How to include the file without specifying a path to it?

You will have to include a path, and use #if defined(OMR_OS_WINDOWS) to distinguish between Windows and Unix paths.

Second query (Windows unimplemented).

When you start implementing for Unix, you will need to include stubs for Windows. Windows won't pick up from common/omrsock.c. @rwy0717 Is this correct?

You can disable the test on Windows by updating the makefile; exclude the omrsockTest object using ifeq (win,$(OMR_HOST_OS)).

Refer to the code and make files in omr/fvtest/porttest.

[caohaley]

@babsingh Is it possible to move contents of unix_include/omrsock.h into omrportsock.h so that the users of the socket API can just include omrportsock.h?

When I include the path to omrsock.h in omrsockTest.cpp, there is the problem of conflicting declaration between omrsock.h 's definition vs. omrportsock.h's forward declaration.

/root/omr/include_core/../port/unix_include/omrsock.h:35:17: error: conflicting declaration 'typedef int32_t OMRSocket'
 typedef int32_t OMRSocket;
                 ^
In file included from /root/omr/include_core/omrport.h:46:0,
                 from /root/omr/fvtest/porttest/omrsockTest.cpp:22:
/root/omr/include_core/omrportsock.h:33:16: error: 'struct OMRSocket' has a previous declaration as 'struct OMRSocket'
 typedef struct OMRSocket *omrsock_socket_t;
                ^

I think it may be because the compiler is confused between struct Type for forward declaration vs Type in typedef. It may think that they are 2 different definitions for the same Type name.

[babsingh]

You are likely to cause redefinition errors as before if you expose struct definitions to the OMR consumer via omrportsock.h.

You are getting the compile error because OMRSocket can't be an int32_t and struct at the same time.

It needs to be

typedef struct OMRSocket {
     int32_t socket;
} OMRSocket;

Let me know if this works.

[caohaley]

Hi Babneet, sorry for the late reply. That method definitely works after I added a struct for OMRSocket and OMRSockAddrStorage.

However, I'm having trouble picturing how that will work for the users because I think the users will need to declare and allocate space for structures like OMRSocket, OMRSockAddrStorage, and OMRAddrInfoNode although they won't directly access the elements inside. They will just pass a pointer to these structures, into the omrsock functions, to perform the operations they need.

In j9sock.c, I think that's how they used the structures: https://github.com/eclipse/openj9/blob/a3a3664e6db6ee632a176c056fe0845bea322a29/runtime/tests/port/j9sockTest.c#L1400

[babsingh]

@caohaley Can you elaborate on your issue with a simple use-case/example?

[caohaley]

@babsingh Basically, I'm still a bit confused on how the users will use our program. For example, in j9sockTest.c, the user allocates struct and pass in the pointer to the struct into the socket API, for example:

j9addrinfo_struct res;
j9addrinfo_t hints;
int32_t length;

j9sock_getaddrinfo_create_hints(&hints, ...);
/* Now hints is pointing to allocated memory in per thread buffer */

/* Pass in pointer to res */
j9sock_getaddrinfo("localhost", hints, &res);

/* To traverse the res, we use j9sock_getaddrinfo_length, j9sock_getaddrinfo_family funtions */
j9sock_getaddrinfo_length(&res, &length);

I think the users may need to access the structs to allocate them locally, even though they won't need to access the elements inside.

[caohaley]

@babsingh However, I was talking to @rwy0717 and Robert brought up that in j9sock.c, the API allocates j9socket_struct for the users in j9sock_socket and frees the memory in j9sock_close.

(Allocation in j9sock_socket: https://github.com/eclipse/openj9/blob/1dd14da6ca1351c8ab42f40b0c7e324c39ab102e/runtime/port/unix/j9sock.c#L3554.)

Therefore, we can maybe follow what j9sock.c did or we can choose one of the two ways.

[babsingh]

@rwy0717 is correct. Similarly, you can hide allocations for other structs in create/open/init methods, and de-allocate them during close/free/destroy methods.

[caohaley]

I see, we could do that. But how would we keep track of what was allocated? For example, in j9close function, you would pass in the j9socket_t to be freed.

[babsingh]

A user can do three things before invoking the omrsock API:

//static allocation
omrsockaddr_struct local_addrStruct;
omrsockaddr_t local_addr = &local_addrStruct;

//dynamic allocation
omrsockaddr_t local_addr = malloc(...);

//no allocation
omrsockaddr_t local_addr = NULL;

Your API should handle all three scenarios. If your API does not handle all three scenarios, then you should clearly state the usage of your API in the description with examples.

how would we keep track of what was allocated?

You can use omrsocket_t as a reference and internally cache the data structs associated with a omrsocket_t in a hash table (or in a similar <key, value> struct). During create/open/init, you create the omrsocket_t (key) and add an entry into the hash table which will contain all the structs associated with the socket. During close/free/destroy, you will query the hash table for the entry and free the structs associated with the omrsocket_t. You can also use an array instead of a hash table, and store the array index within omrsocket_t to point to the associated structs.

[caohaley]

I see that makes a lot of sense, thank you Babneet! I will look into it more.

[caohaley]

@babsingh Would it be helpful if we made the definition of the OMRSocket struct:

typedef OMRSocket{
    int socket;
    int curMemoryIndex;
    void * memoryManagement[30];
} OMRSocket;

When we allocate the OMRSocket struct for the user, we set curMemoryIndex to 0, and initialize all elements of memoryManagement to NULL.

When we need to allocate memory we copy the pointer to the block of memory we allocate, to the memoryManagement array and increment curMemoryIndex. If the user has already allocated memory for a struct, we simply just don't allocate the memory and don't add it to the memoryManagement array.

At the very end, we free the pointers to the memory one by one.

[babsingh]

@caohaley What do you want to store for every OMRSocket? void * memoryManagement[30] allocates memory for 30 void pointers. What do you plan to store in them?

[caohaley]

I was thinking we could store the pointers to all the structs that we allocate for the user. I used a maximum of 30 allocations per socket_t that for the user, as an example.

Or we could use the hash table and it would look more clean. However, it may require more lines of programming.

[caohaley]

@babsingh For the static and dynamic allocation method, will we still have the user include both omrsock.h as well as omrportsock.h, where omrsock.h defines the structs and omrportsock.h defining the pointers? It just seems to me a little inconvenient for the users to find the exact path to omrsock.h when including the header, where we could directly include the definition of the structs in omrportsock.h.

//static allocation
omrsockaddr_struct local_addrStruct;
omrsockaddr_t local_addr = &local_addrStruct;

//dynamic allocation
omrsockaddr_t local_addr = malloc(...);

[babsingh]

For the static and dynamic allocation method

You can just do the allocation for them in your API, and mention usage of the API with examples in the description. Then, users won't have to worry about allocations and including header files.

We could store the pointers to all the structs that we allocate for the user

You need to identify all the structs per socket. Create a struct like:

typedef OMRSocketMeta{
    struct_1;
    struct_2;
    struct_3;
    ...;
} OMRSocketMeta;

typedef OMRSocket{
    int socket;
    struct OMRSocketMeta* meta;
} OMRSocket;

Allocate/initialize meta at the start, and de-allocate meta at the end. If there are global structures (shared by the entire socket library), then store them in J9PortLibrary; you can allocate and de-allocate them in the startup/shutdown methods. There are many other ways to implement it. Using an array of void * memoryManagement[30] is not a good strategy; you may not use all the array elements (unused memory) or the 30 elements may not be sufficient; also, you can't identify type of array element (void *).

[babsingh]

lgtm.

[babsingh]

lgtm.

[rwy7]

@genie-omr build all

[caohaley]

@rwy0717 The checks has passed.