Quantcast

Re: Usage of GL on Android not possible because of javax

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Usage of GL on Android not possible because of javax

hbrands
Administrator
Hi Rob et.al.,

with these changes applied in production we now sporadically observe exceptions like this:

java.lang.NullPointerException
    at ca.odell.glazedlists.matchers.AbstractMatcherEditorListenerSupport.fireChangedMatcher(AbstractMatcherEditorListenerSupport.java:45)
    at ca.odell.glazedlists.matchers.ThreadedMatcherEditor$DrainMatcherEventQueueRunnable.run(ThreadedMatcherEditor.java:249)
    at java.lang.Thread.run(Unknown Source)

I think it is related to the changes indicated below.
My assumption is that the code is not thread-safe anymore. (I'm pretty sure that we don't add null as MatcherEditorListener.)
Shouldn't we use CopyOnWriteArrayList or write a replacement for the old EventListenerList?

WDYT?

Thanks,
Holger


2016-08-29 21:37 GMT+02:00 Rob Eden <[hidden email]>:
Oh, sorry... I missed that AbstractMatcherEditorListenerSupport was the class using it.

I put in a fix which just uses ArrayList instead (no real need for the dependency):

I think Holger normally releases snapshots, so maybe he can build one for you if need be?

On Mon, Aug 29, 2016 at 1:19 PM Fabian Zeindl <[hidden email]> wrote:
The problem is that it uses javax.swing.events.EventListenerList 
event when no swing is used.

So that needs to be replaced.


On 29.08.2016, at 19:37 , Rob Eden <[hidden email]> wrote:

Ultimately stuff should be broken out into separate modules. However, for the short term you could probably just use ProGuard to rip out stuff you don't want.

On Mon, Aug 29, 2016 at 8:35 AM <[hidden email]> wrote:
Hi,

 I wanted to use GL on Android, but there are some imports from swing
in the core (notably: javax.swing.event.EventListenerList in
AbstractMatcherEditorListenerSupport.java), that makes it unusable.

Would you consider patching this?

Here is a code-search:
https://github.com/glazedlists/glazedlists/search?utf8=%E2%9C%93&q=%22i
mport+javax%22+path%3A%22source%2Fca%2Fodell%2Fglazedlists%22+-path%3A%
22source%2Fca%2Fodell%2Fglazedlists%2Fswing%22++-path%3A%22source%2Fca%
2Fodell%2Fglazedlists%2Fimpl%2Fswing%22&type=Code



Thank you
Fabian


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Usage of GL on Android not possible because of javax

robeden
Woah, have to remember back to that change and I'm traveling at the moment so I don't have good access to code...

I think at the time I thought that would be in a (thread) locked context, but I suppose you're right that it's probably not required (do the docs detail requirements or non-requirement?).

I'm not sure that performance matters too much in this scenario, but my gut is that you're probably on the right track with CopyOnWriteArrayList (versus something like a synchronized ArrayList) since lock-free on the reads is high priority.

Do you have a reliable test case? I've been running with that change for a while and haven't seen the NPE... though I suspect my listener changes are done in locks.


On Fri, Feb 3, 2017 at 8:06 AM Holger Brands <[hidden email]> wrote:
Hi Rob et.al.,

with these changes applied in production we now sporadically observe exceptions like this:

java.lang.NullPointerException
    at ca.odell.glazedlists.matchers.AbstractMatcherEditorListenerSupport.fireChangedMatcher(AbstractMatcherEditorListenerSupport.java:45)
    at ca.odell.glazedlists.matchers.ThreadedMatcherEditor$DrainMatcherEventQueueRunnable.run(ThreadedMatcherEditor.java:249)
    at java.lang.Thread.run(Unknown Source)

I think it is related to the changes indicated below.
My assumption is that the code is not thread-safe anymore. (I'm pretty sure that we don't add null as MatcherEditorListener.)
Shouldn't we use CopyOnWriteArrayList or write a replacement for the old EventListenerList?

WDYT?

Thanks,
Holger


2016-08-29 21:37 GMT+02:00 Rob Eden <[hidden email]>:
Oh, sorry... I missed that AbstractMatcherEditorListenerSupport was the class using it.

I put in a fix which just uses ArrayList instead (no real need for the dependency):

I think Holger normally releases snapshots, so maybe he can build one for you if need be?

On Mon, Aug 29, 2016 at 1:19 PM Fabian Zeindl <[hidden email]> wrote:
The problem is that it uses javax.swing.events.EventListenerList 
event when no swing is used.

So that needs to be replaced.


On 29.08.2016, at 19:37 , Rob Eden <[hidden email]> wrote:

Ultimately stuff should be broken out into separate modules. However, for the short term you could probably just use ProGuard to rip out stuff you don't want.

On Mon, Aug 29, 2016 at 8:35 AM <[hidden email]> wrote:
Hi,

 I wanted to use GL on Android, but there are some imports from swing
in the core (notably: javax.swing.event.EventListenerList in
AbstractMatcherEditorListenerSupport.java), that makes it unusable.

Would you consider patching this?

Here is a code-search:
https://github.com/glazedlists/glazedlists/search?utf8=%E2%9C%93&q=%22i
mport+javax%22+path%3A%22source%2Fca%2Fodell%2Fglazedlists%22+-path%3A%
22source%2Fca%2Fodell%2Fglazedlists%2Fswing%22++-path%3A%22source%2Fca%
2Fodell%2Fglazedlists%2Fimpl%2Fswing%22&type=Code



Thank you
Fabian


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Usage of GL on Android not possible because of javax

hbrands
Administrator
In the context of ThreadedMatcherEditor, it's a concurrent scenario:

addMatcherEditorListener / removeMatcherEditorListener is called by the thread which creates / disposes the FilterList.
fireChangedMatcher is called by the "MatcherQueueThread" of ThreadedMatcherEditor.

We've had about twenty occurences of this issue within a week or so.
But I don't have a reliable test case.
If we don't want to use EventListenerList I think I'll go ahead with the CopyOnWriteArrayList.

Thanks,
Holger



2017-02-04 4:18 GMT+01:00 Rob Eden <[hidden email]>:
Woah, have to remember back to that change and I'm traveling at the moment so I don't have good access to code...

I think at the time I thought that would be in a (thread) locked context, but I suppose you're right that it's probably not required (do the docs detail requirements or non-requirement?).

I'm not sure that performance matters too much in this scenario, but my gut is that you're probably on the right track with CopyOnWriteArrayList (versus something like a synchronized ArrayList) since lock-free on the reads is high priority.

Do you have a reliable test case? I've been running with that change for a while and haven't seen the NPE... though I suspect my listener changes are done in locks.



On Fri, Feb 3, 2017 at 8:06 AM Holger Brands <[hidden email]> wrote:
Hi Rob et.al.,

with these changes applied in production we now sporadically observe exceptions like this:

java.lang.NullPointerException
    at ca.odell.glazedlists.matchers.AbstractMatcherEditorListenerSupport.fireChangedMatcher(AbstractMatcherEditorListenerSupport.java:45)
    at ca.odell.glazedlists.matchers.ThreadedMatcherEditor$DrainMatcherEventQueueRunnable.run(ThreadedMatcherEditor.java:249)
    at java.lang.Thread.run(Unknown Source)

I think it is related to the changes indicated below.
My assumption is that the code is not thread-safe anymore. (I'm pretty sure that we don't add null as MatcherEditorListener.)
Shouldn't we use CopyOnWriteArrayList or write a replacement for the old EventListenerList?

WDYT?

Thanks,
Holger


2016-08-29 21:37 GMT+02:00 Rob Eden <[hidden email]>:
Oh, sorry... I missed that AbstractMatcherEditorListenerSupport was the class using it.

I put in a fix which just uses ArrayList instead (no real need for the dependency):

I think Holger normally releases snapshots, so maybe he can build one for you if need be?

On Mon, Aug 29, 2016 at 1:19 PM Fabian Zeindl <[hidden email]> wrote:
The problem is that it uses javax.swing.events.EventListenerList 
event when no swing is used.

So that needs to be replaced.


On 29.08.2016, at 19:37 , Rob Eden <[hidden email]> wrote:

Ultimately stuff should be broken out into separate modules. However, for the short term you could probably just use ProGuard to rip out stuff you don't want.

On Mon, Aug 29, 2016 at 8:35 AM <[hidden email]> wrote:
Hi,

 I wanted to use GL on Android, but there are some imports from swing
in the core (notably: javax.swing.event.EventListenerList in
AbstractMatcherEditorListenerSupport.java), that makes it unusable.

Would you consider patching this?

Here is a code-search:
https://github.com/glazedlists/glazedlists/search?utf8=%E2%9C%93&q=%22i
mport+javax%22+path%3A%22source%2Fca%2Fodell%2Fglazedlists%22+-path%3A%
22source%2Fca%2Fodell%2Fglazedlists%2Fswing%22++-path%3A%22source%2Fca%
2Fodell%2Fglazedlists%2Fimpl%2Fswing%22&type=Code



Thank you
Fabian



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Usage of GL on Android not possible because of javax

robeden
Somehow in all my wandering a through the GL API I'd never come across ThreadedMatcherEditor. I'll have to examine the implementation more when I have time. I worry about back pressure handling and such... but that's not relevant to this problem.

Anyway, go for it with the change. Sorry for the breakage. Maybe we can get a unit test put together later to ensure proper handling in the future.


On Sat, Feb 4, 2017 at 8:07 AM Holger Brands <[hidden email]> wrote:
In the context of ThreadedMatcherEditor, it's a concurrent scenario:

addMatcherEditorListener / removeMatcherEditorListener is called by the thread which creates / disposes the FilterList.
fireChangedMatcher is called by the "MatcherQueueThread" of ThreadedMatcherEditor.

We've had about twenty occurences of this issue within a week or so.
But I don't have a reliable test case.
If we don't want to use EventListenerList I think I'll go ahead with the CopyOnWriteArrayList.

Thanks,
Holger



2017-02-04 4:18 GMT+01:00 Rob Eden <[hidden email]>:
Woah, have to remember back to that change and I'm traveling at the moment so I don't have good access to code...

I think at the time I thought that would be in a (thread) locked context, but I suppose you're right that it's probably not required (do the docs detail requirements or non-requirement?).

I'm not sure that performance matters too much in this scenario, but my gut is that you're probably on the right track with CopyOnWriteArrayList (versus something like a synchronized ArrayList) since lock-free on the reads is high priority.

Do you have a reliable test case? I've been running with that change for a while and haven't seen the NPE... though I suspect my listener changes are done in locks.



On Fri, Feb 3, 2017 at 8:06 AM Holger Brands <[hidden email]> wrote:
Hi Rob et.al.,

with these changes applied in production we now sporadically observe exceptions like this:

java.lang.NullPointerException
    at ca.odell.glazedlists.matchers.AbstractMatcherEditorListenerSupport.fireChangedMatcher(AbstractMatcherEditorListenerSupport.java:45)
    at ca.odell.glazedlists.matchers.ThreadedMatcherEditor$DrainMatcherEventQueueRunnable.run(ThreadedMatcherEditor.java:249)
    at java.lang.Thread.run(Unknown Source)

I think it is related to the changes indicated below.
My assumption is that the code is not thread-safe anymore. (I'm pretty sure that we don't add null as MatcherEditorListener.)
Shouldn't we use CopyOnWriteArrayList or write a replacement for the old EventListenerList?

WDYT?

Thanks,
Holger


2016-08-29 21:37 GMT+02:00 Rob Eden <[hidden email]>:
Oh, sorry... I missed that AbstractMatcherEditorListenerSupport was the class using it.

I put in a fix which just uses ArrayList instead (no real need for the dependency):

I think Holger normally releases snapshots, so maybe he can build one for you if need be?

On Mon, Aug 29, 2016 at 1:19 PM Fabian Zeindl <[hidden email]> wrote:
The problem is that it uses javax.swing.events.EventListenerList 
event when no swing is used.

So that needs to be replaced.


On 29.08.2016, at 19:37 , Rob Eden <[hidden email]> wrote:

Ultimately stuff should be broken out into separate modules. However, for the short term you could probably just use ProGuard to rip out stuff you don't want.

On Mon, Aug 29, 2016 at 8:35 AM <[hidden email]> wrote:
Hi,

 I wanted to use GL on Android, but there are some imports from swing
in the core (notably: javax.swing.event.EventListenerList in
AbstractMatcherEditorListenerSupport.java), that makes it unusable.

Would you consider patching this?

Here is a code-search:
https://github.com/glazedlists/glazedlists/search?utf8=%E2%9C%93&q=%22i
mport+javax%22+path%3A%22source%2Fca%2Fodell%2Fglazedlists%22+-path%3A%
22source%2Fca%2Fodell%2Fglazedlists%2Fswing%22++-path%3A%22source%2Fca%
2Fodell%2Fglazedlists%2Fimpl%2Fswing%22&type=Code



Thank you
Fabian



Loading...