Bug and patch - Heavy device io can hang

[Sergey Nazarkin]

Craig, 

The patch looks OK (at least for the moment, the whole signal/wait subsystem requires refactoring).  Have you signed Oracle Contributed Agreement (http://openjdk.java.net/contribute/)?


Sergey Nazarkin




> On 25 May 2018, at 18:32, Sergey Nazarkin <snazarkin at azul.com> wrote:
> 
> Hi, Craig!
> 
> Thanks for the fix. I’ll take a look on Monday
> 
> 
> Sergey Nazarkin
> 
> 
> 
> 
>> On 25 May 2018, at 07:57, Craig White <cwhite102 at gmail.com> wrote:
>> 
>> I'm not sure if this library is being maintained or not, but I found it was
>> quite useful to get some boards going with a pi!
>> 
>> 
>> 
>> I ran into a hang situation with heavy writes to an I2C device, and traced
>> the issue to improper mutex/condition code in dio_common.cpp.
>> 
>> It was very reproducible and evident that the waitForCondition method could
>> miss the signal from generateSignal() and just get stuck there waiting.
>> 
>> 
>> 
>> Craig
>> 
>> 
>> 
>> Here's a patch to fix the issue:
>> 
>> 
>> 
>> 
>> 
>> @@ -279,4 +279,10 @@
>> 
>>        if (sig != NULL) {
>> 
>>            javautil_list_add(signals, sig);
>> 
>> +            // Need to lock the mutex associated with the condition
>> 
>> +            // otherwise we could miss the signal
>> 
>> +            // This is done while holding the 'master' signalMutex
>> 
>> +
>> javacall_os_mutex_lock(javacall_os_cond_get_mutex(sig->condition));
>> 
>> +
>> 
>> +            // Now release the master lock
>> 
>>            javacall_os_mutex_unlock(signalMutex);
>> 
>> @@ -282,2 +288,4 @@
>> 
>>            javacall_os_mutex_unlock(signalMutex);
>> 
>> +
>> 
>> +            // Wait on the condition (releases the condition mutex while
>> waiting)
>> 
>>            javacall_os_cond_wait(sig->condition, timeout);
>> 
>> @@ -283,4 +291,9 @@
>> 
>>            javacall_os_cond_wait(sig->condition, timeout);
>> 
>> +            // Technically, this should be in a loop for spurious wakes,
>> but
>> 
>> +            // I'm not familiar enough with the code for what the loop
>> would check
>> 
>> +
>> 
>> +            // Unlock the condition mutex
>> 
>> +
>> javacall_os_mutex_unlock(javacall_os_cond_get_mutex(sig->condition));
>> 
>>        }
>> 
>>    } else {
>> 
>>        javacall_os_mutex_unlock(signalMutex);
>> 
>> @@ -313,4 +326,9 @@
>> 
>>    signal* sig = findTarget(signalType, signalTarget);
>> 
>>    if (sig != NULL) {
>> 
>>        sig->parameter = signalParameter;
>> 
>> +        
>> 
>> +        // Need to lock the mutex associated with the condition
>> 
>> +        // or waitForSignal could miss the signal
>> 
>> +        javacall_os_mutex_lock( javacall_os_cond_get_mutex(sig->condition)
>> );
>> 
>> +
>> 
>>        javacall_os_cond_signal(sig->condition);
>> 
>> @@ -316,4 +334,6 @@
>> 
>>        javacall_os_cond_signal(sig->condition);
>> 
>> +        
>> 
>> +        javacall_os_mutex_unlock(
>> javacall_os_cond_get_mutex(sig->condition) );
>> 
>>    } else {
>> 
>>        // signal is being sent before wait started
>> 
>>        // create the signal and add it to the list
>> 
>> 
>