Bug and patch - Heavy device io can hang
Sergey Nazarkin
snazarkin at azul.com
Thu May 31 14:24:40 UTC 2018
To accept the patch I need your (or your employer) OCA. Are you individual contributor or this is part of job activity?
Sergey Nazarkin
> On 31 May 2018, at 17:18, Craig White <cwhite102 at gmail.com> wrote:
>
> Thanks Sergey.
> I haven't signed the contributor agreement yet. Do you need this to accept the patch?
> I usually just use my co-worker (who has some level of contributor status) as the interface to Oracle/OpenJDK when we find issues.
> Craig
>
>
> -----Original Message-----
> From: Sergey Nazarkin [mailto:snazarkin at azul.com]
> Sent: Thursday, May 31, 2018 10:07 AM
> To: Craig White
> Cc: dio-dev at openjdk.java.net
> Subject: Re: Bug and patch - Heavy device io can hang
>
> 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
>>>
>>>
>>
>
More information about the dio-dev
mailing list